Changes since v1: * Re-implement with a new magic number instead of piggybacking on ETMv4 * Improve comments and function name around cs_etm_decoder__get_etmv4_arch_ver() * Add a warning for unrecognised magic numbers * Split typo fix into new commit * Add Leo's reviewed-by tags * Create a new struct for ETE config (cs_ete_trace_params) instead of re-using ETMv4 config
Applies to perf/core f3c33cbd922 Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-ete-v2
James Clark (9): perf cs-etm: Refactor initialisation of decoder params. perf cs-etm: Initialise architecture based on TRCIDR1 perf cs-etm: Refactor out ETMv4 header saving perf cs-etm: Save TRCDEVARCH register perf cs-etm: Fix typo perf cs-etm: Update OpenCSD decoder for ETE perf cs-etm: Create ETE decoder perf cs-etm: Print the decoder name perf cs-etm: Show a warning for an unknown magic number
tools/build/feature/test-libopencsd.c | 4 +- tools/perf/arch/arm/util/cs-etm.c | 97 ++++++++---- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 148 ++++++++---------- .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 13 ++ tools/perf/util/cs-etm.c | 43 ++++- tools/perf/util/cs-etm.h | 10 ++ 6 files changed, 200 insertions(+), 115 deletions(-)
The initialisation of the decoder params is duplicated between creation of the packet printer and packet decoder. Put them both into one function so that future changes only need to be made in one place.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com --- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 99 +++++-------------- 1 file changed, 25 insertions(+), 74 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 84cca3fc05a5..187c038caa19 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -227,55 +227,6 @@ cs_etm_decoder__init_raw_frame_logging( } #endif
-static int cs_etm_decoder__create_packet_printer(struct cs_etm_decoder *decoder, - const char *decoder_name, - void *trace_config) -{ - u8 csid; - - if (ocsd_dt_create_decoder(decoder->dcd_tree, decoder_name, - OCSD_CREATE_FLG_PACKET_PROC, - trace_config, &csid)) - return -1; - - if (ocsd_dt_set_pkt_protocol_printer(decoder->dcd_tree, csid, 0)) - return -1; - - return 0; -} - -static int -cs_etm_decoder__create_etm_packet_printer(struct cs_etm_trace_params *t_params, - struct cs_etm_decoder *decoder) -{ - const char *decoder_name; - ocsd_etmv3_cfg config_etmv3; - ocsd_etmv4_cfg trace_config_etmv4; - void *trace_config; - - switch (t_params->protocol) { - case CS_ETM_PROTO_ETMV3: - case CS_ETM_PROTO_PTM: - cs_etm_decoder__gen_etmv3_config(t_params, &config_etmv3); - decoder_name = (t_params->protocol == CS_ETM_PROTO_ETMV3) ? - OCSD_BUILTIN_DCD_ETMV3 : - OCSD_BUILTIN_DCD_PTM; - trace_config = &config_etmv3; - break; - case CS_ETM_PROTO_ETMV4i: - cs_etm_decoder__gen_etmv4_config(t_params, &trace_config_etmv4); - decoder_name = OCSD_BUILTIN_DCD_ETMV4I; - trace_config = &trace_config_etmv4; - break; - default: - return -1; - } - - return cs_etm_decoder__create_packet_printer(decoder, - decoder_name, - trace_config); -} - static ocsd_datapath_resp_t cs_etm_decoder__do_soft_timestamp(struct cs_etm_queue *etmq, struct cs_etm_packet_queue *packet_queue, @@ -632,9 +583,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( return resp; }
-static int cs_etm_decoder__create_etm_packet_decoder( - struct cs_etm_trace_params *t_params, - struct cs_etm_decoder *decoder) +static int +cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, + struct cs_etm_trace_params *t_params, + struct cs_etm_decoder *decoder) { const char *decoder_name; ocsd_etmv3_cfg config_etmv3; @@ -660,31 +612,30 @@ static int cs_etm_decoder__create_etm_packet_decoder( return -1; }
- if (ocsd_dt_create_decoder(decoder->dcd_tree, - decoder_name, - OCSD_CREATE_FLG_FULL_DECODER, - trace_config, &csid)) - return -1; + if (d_params->operation == CS_ETM_OPERATION_DECODE) { + if (ocsd_dt_create_decoder(decoder->dcd_tree, + decoder_name, + OCSD_CREATE_FLG_FULL_DECODER, + trace_config, &csid)) + return -1;
- if (ocsd_dt_set_gen_elem_outfn(decoder->dcd_tree, - cs_etm_decoder__gen_trace_elem_printer, - decoder)) - return -1; + if (ocsd_dt_set_gen_elem_outfn(decoder->dcd_tree, + cs_etm_decoder__gen_trace_elem_printer, + decoder)) + return -1;
- return 0; -} + return 0; + } else if (d_params->operation == CS_ETM_OPERATION_PRINT) { + if (ocsd_dt_create_decoder(decoder->dcd_tree, decoder_name, + OCSD_CREATE_FLG_PACKET_PROC, + trace_config, &csid)) + return -1;
-static int -cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, - struct cs_etm_trace_params *t_params, - struct cs_etm_decoder *decoder) -{ - if (d_params->operation == CS_ETM_OPERATION_PRINT) - return cs_etm_decoder__create_etm_packet_printer(t_params, - decoder); - else if (d_params->operation == CS_ETM_OPERATION_DECODE) - return cs_etm_decoder__create_etm_packet_decoder(t_params, - decoder); + if (ocsd_dt_set_pkt_protocol_printer(decoder->dcd_tree, csid, 0)) + return -1; + + return 0; + }
return -1; }
On 06/08/2021 14:41, James Clark wrote:
The initialisation of the decoder params is duplicated between creation of the packet printer and packet decoder. Put them both into one function so that future changes only need to be made in one place.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Em Fri, Sep 03, 2021 at 09:52:14AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
The initialisation of the decoder params is duplicated between creation of the packet printer and packet decoder. Put them both into one function so that future changes only need to be made in one place.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 12:05, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 09:52:14AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
The initialisation of the decoder params is duplicated between creation of the packet printer and packet decoder. Put them both into one function so that future changes only need to be made in one place.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Currently the architecture is hard coded as ARCH_V8, but from ETMv4.4 onwards this should be ARCH_AA64.
Signed-off-by: James Clark james.clark@arm.com --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 16 +++++++++++++++- 1 file changed, 15 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 187c038caa19..787b19642e78 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,20 @@ 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_etmv4_arch_ver(u32 reg_idr1) +{ + /* + * For ETMv4 if the trace minor version is 4 or more then we can assume + * the architecture is ARCH_AA64 rather than just V8. + * ARCH_V8 = V8 architecture + * ARCH_AA64 = Min v8r3 plus additional AA64 PE features + */ + return TRCIDR1_TRCARCHMIN(reg_idr1) >= 4 ? ARCH_AA64 : ARCH_V8; +} + static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, ocsd_etmv4_cfg *config) { @@ -140,7 +154,7 @@ static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, config->reg_idr11 = 0; config->reg_idr12 = 0; config->reg_idr13 = 0; - config->arch_ver = ARCH_V8; + config->arch_ver = cs_etm_decoder__get_etmv4_arch_ver(params->etmv4.reg_idr1); config->core_prof = profile_CortexA; }
On Fri, Aug 06, 2021 at 02:41:02PM +0100, James Clark wrote:
Currently the architecture is hard coded as ARCH_V8, but from ETMv4.4 onwards this should be ARCH_AA64.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 16 +++++++++++++++- 1 file changed, 15 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 187c038caa19..787b19642e78 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,20 @@ 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_etmv4_arch_ver(u32 reg_idr1) +{
- /*
* For ETMv4 if the trace minor version is 4 or more then we can assume* the architecture is ARCH_AA64 rather than just V8.* ARCH_V8 = V8 architecture* ARCH_AA64 = Min v8r3 plus additional AA64 PE features*/- return TRCIDR1_TRCARCHMIN(reg_idr1) >= 4 ? ARCH_AA64 : ARCH_V8;
+}
static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, ocsd_etmv4_cfg *config) { @@ -140,7 +154,7 @@ static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, config->reg_idr11 = 0; config->reg_idr12 = 0; config->reg_idr13 = 0;
- config->arch_ver = ARCH_V8;
- config->arch_ver = cs_etm_decoder__get_etmv4_arch_ver(params->etmv4.reg_idr1); config->core_prof = profile_CortexA;
}
Reviewed-by: Leo Yan leo.yan@linaro.org
2.28.0
On 06/08/2021 14:41, James Clark wrote:
Currently the architecture is hard coded as ARCH_V8, but from ETMv4.4 onwards this should be ARCH_AA64.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 16 +++++++++++++++- 1 file changed, 15 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 187c038caa19..787b19642e78 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,20 @@ 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)
minor nit: Please add a blank line here
+static enum _ocsd_arch_version cs_etm_decoder__get_etmv4_arch_ver(u32 reg_idr1) +{
- /*
* For ETMv4 if the trace minor version is 4 or more then we can assume* the architecture is ARCH_AA64 rather than just V8.* ARCH_V8 = V8 architecture* ARCH_AA64 = Min v8r3 plus additional AA64 PE features
Does this need to be >= 3 then, to be accurate ?
The comment "reads", minimum v8.3 and any additional features.
Otherwise looks good to me.
Suzuki
Em Fri, Sep 03, 2021 at 09:55:37AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
Currently the architecture is hard coded as ARCH_V8, but from ETMv4.4 onwards this should be ARCH_AA64.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 16 +++++++++++++++- 1 file changed, 15 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 187c038caa19..787b19642e78 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,20 @@ 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)
minor nit: Please add a blank line here
Fixed it
+static enum _ocsd_arch_version cs_etm_decoder__get_etmv4_arch_ver(u32 reg_idr1) +{
- /*
* For ETMv4 if the trace minor version is 4 or more then we can assume* the architecture is ARCH_AA64 rather than just V8.* ARCH_V8 = V8 architecture* ARCH_AA64 = Min v8r3 plus additional AA64 PE featuresDoes this need to be >= 3 then, to be accurate ?
The comment "reads", minimum v8.3 and any additional features.
Otherwise looks good to me.
Didn't have enough coffee, I think, can you please provide this as a patch? :)
- Arnaldo
On Fri, 3 Sept 2021 at 12:07, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 09:55:37AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
Currently the architecture is hard coded as ARCH_V8, but from ETMv4.4 onwards this should be ARCH_AA64.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 16 +++++++++++++++- 1 file changed, 15 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 187c038caa19..787b19642e78 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,20 @@ 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)
minor nit: Please add a blank line here
Fixed it
+static enum _ocsd_arch_version cs_etm_decoder__get_etmv4_arch_ver(u32 reg_idr1) +{
- /*
- For ETMv4 if the trace minor version is 4 or more then we can assume
- the architecture is ARCH_AA64 rather than just V8.
- ARCH_V8 = V8 architecture
- ARCH_AA64 = Min v8r3 plus additional AA64 PE features
Does this need to be >= 3 then, to be accurate ?
The comment "reads", minimum v8.3 and any additional features.
Otherwise looks good to me.
Behaviour correctly matches OpenCSD decoder requirements.
Reviewed-by: Mike Leach mike.leach@linaro.org
Didn't have enough coffee, I think, can you please provide this as a patch? :)
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Extract a function for saving the ETMv4 header because this will be used for ETE in a later commit.
Signed-off-by: James Clark james.clark@arm.com --- tools/perf/arch/arm/util/cs-etm.c | 46 +++++++++++++++---------------- 1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 85168d87b2d7..ecb6fa55a210 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -607,6 +607,28 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; }
+static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) +{ + struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr); + struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; + + /* Get trace configuration register */ + data[CS_ETMV4_TRCCONFIGR] = cs_etmv4_get_config(itr); + /* Get traceID from the framework */ + data[CS_ETMV4_TRCTRACEIDR] = coresight_get_trace_id(cpu); + /* Get read-only information from sysFS */ + data[CS_ETMV4_TRCIDR0] = cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETMV4_TRCIDR0]); + data[CS_ETMV4_TRCIDR1] = cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETMV4_TRCIDR1]); + data[CS_ETMV4_TRCIDR2] = cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETMV4_TRCIDR2]); + data[CS_ETMV4_TRCIDR8] = cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETMV4_TRCIDR8]); + data[CS_ETMV4_TRCAUTHSTATUS] = cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS]); +} + static void cs_etm_get_metadata(int cpu, u32 *offset, struct auxtrace_record *itr, struct perf_record_auxtrace_info *info) @@ -620,29 +642,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, /* first see what kind of tracer this cpu is affined to */ if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; - /* Get trace configuration register */ - info->priv[*offset + CS_ETMV4_TRCCONFIGR] = - cs_etmv4_get_config(itr); - /* Get traceID from the framework */ - info->priv[*offset + CS_ETMV4_TRCTRACEIDR] = - coresight_get_trace_id(cpu); - /* Get read-only information from sysFS */ - info->priv[*offset + CS_ETMV4_TRCIDR0] = - cs_etm_get_ro(cs_etm_pmu, cpu, - metadata_etmv4_ro[CS_ETMV4_TRCIDR0]); - info->priv[*offset + CS_ETMV4_TRCIDR1] = - cs_etm_get_ro(cs_etm_pmu, cpu, - metadata_etmv4_ro[CS_ETMV4_TRCIDR1]); - info->priv[*offset + CS_ETMV4_TRCIDR2] = - cs_etm_get_ro(cs_etm_pmu, cpu, - metadata_etmv4_ro[CS_ETMV4_TRCIDR2]); - info->priv[*offset + CS_ETMV4_TRCIDR8] = - cs_etm_get_ro(cs_etm_pmu, cpu, - metadata_etmv4_ro[CS_ETMV4_TRCIDR8]); - info->priv[*offset + CS_ETMV4_TRCAUTHSTATUS] = - cs_etm_get_ro(cs_etm_pmu, cpu, - metadata_etmv4_ro - [CS_ETMV4_TRCAUTHSTATUS]); + cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
/* How much space was used */ increment = CS_ETMV4_PRIV_MAX;
On Fri, Aug 06, 2021 at 02:41:03PM +0100, James Clark wrote:
Extract a function for saving the ETMv4 header because this will be used for ETE in a later commit.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
tools/perf/arch/arm/util/cs-etm.c | 46 +++++++++++++++---------------- 1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 85168d87b2d7..ecb6fa55a210 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -607,6 +607,28 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; } +static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) +{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- /* Get trace configuration register */
- data[CS_ETMV4_TRCCONFIGR] = cs_etmv4_get_config(itr);
- /* Get traceID from the framework */
- data[CS_ETMV4_TRCTRACEIDR] = coresight_get_trace_id(cpu);
- /* Get read-only information from sysFS */
- data[CS_ETMV4_TRCIDR0] = cs_etm_get_ro(cs_etm_pmu, cpu,
metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);- data[CS_ETMV4_TRCIDR1] = cs_etm_get_ro(cs_etm_pmu, cpu,
metadata_etmv4_ro[CS_ETMV4_TRCIDR1]);- data[CS_ETMV4_TRCIDR2] = cs_etm_get_ro(cs_etm_pmu, cpu,
metadata_etmv4_ro[CS_ETMV4_TRCIDR2]);- data[CS_ETMV4_TRCIDR8] = cs_etm_get_ro(cs_etm_pmu, cpu,
metadata_etmv4_ro[CS_ETMV4_TRCIDR8]);- data[CS_ETMV4_TRCAUTHSTATUS] = cs_etm_get_ro(cs_etm_pmu, cpu,
metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS]);+}
static void cs_etm_get_metadata(int cpu, u32 *offset, struct auxtrace_record *itr, struct perf_record_auxtrace_info *info) @@ -620,29 +642,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, /* first see what kind of tracer this cpu is affined to */ if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic;
/* Get trace configuration register */info->priv[*offset + CS_ETMV4_TRCCONFIGR] =cs_etmv4_get_config(itr);/* Get traceID from the framework */info->priv[*offset + CS_ETMV4_TRCTRACEIDR] =coresight_get_trace_id(cpu);/* Get read-only information from sysFS */info->priv[*offset + CS_ETMV4_TRCIDR0] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);info->priv[*offset + CS_ETMV4_TRCIDR1] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETMV4_TRCIDR1]);info->priv[*offset + CS_ETMV4_TRCIDR2] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETMV4_TRCIDR2]);info->priv[*offset + CS_ETMV4_TRCIDR8] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETMV4_TRCIDR8]);info->priv[*offset + CS_ETMV4_TRCAUTHSTATUS] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS]);
cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);/* How much space was used */ increment = CS_ETMV4_PRIV_MAX; -- 2.28.0
On 06/08/2021 14:41, James Clark wrote:
Extract a function for saving the ETMv4 header because this will be used for ETE in a later commit.
Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 09:57, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 06/08/2021 14:41, James Clark wrote:
Extract a function for saving the ETMv4 header because this will be used for ETE in a later commit.
Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
When ETE is present save the TRCDEVARCH register and set a new magic number. It will be used to configure the decoder in a later commit.
Old versions of perf will not be able to open files with this new magic number, but old files will still work with newer versions of perf.
Signed-off-by: James Clark james.clark@arm.com --- tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++---- tools/perf/util/cs-etm.c | 16 ++++++++-- tools/perf/util/cs-etm.h | 10 +++++++ 3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ecb6fa55a210..e3500b79d972 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = { [CS_ETM_ETMIDR] = "mgmt/etmidr", };
-static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = { +static const char * const metadata_etmv4_ro[] = { [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0", [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1", [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2", [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8", [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus", + [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch" };
static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu); +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu);
static int cs_etm_set_context_id(struct auxtrace_record *itr, struct evsel *evsel, int cpu) @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, struct evlist *evlist __maybe_unused) { int i; - int etmv3 = 0, etmv4 = 0; + int etmv3 = 0, etmv4 = 0, ete = 0; struct perf_cpu_map *event_cpus = evlist->core.cpus; struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
@@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, !cpu_map__has(online_cpus, i)) continue;
- if (cs_etm_is_etmv4(itr, i)) + if (cs_etm_is_ete(itr, i)) + ete++; + else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++; @@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, if (!cpu_map__has(online_cpus, i)) continue;
- if (cs_etm_is_etmv4(itr, i)) + if (cs_etm_is_ete(itr, i)) + ete++; + else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++; @@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, perf_cpu_map__put(online_cpus);
return (CS_ETM_HEADER_SIZE + + (ete * CS_ETE_PRIV_SIZE) + (etmv4 * CS_ETMV4_PRIV_SIZE) + (etmv3 * CS_ETMV3_PRIV_SIZE)); } @@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; }
+#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) + +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu) +{ + struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr); + struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; + int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_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; +} + static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) { struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr); @@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
/* first see what kind of tracer this cpu is affined to */ - if (cs_etm_is_etmv4(itr, cpu)) { + if (cs_etm_is_ete(itr, cpu)) { + magic = __perf_cs_ete_magic; + /* ETE uses the same registers as ETMv4 plus TRCDEVARCH */ + cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu); + info->priv[*offset + CS_ETE_TRCDEVARCH] = + cs_etm_get_ro(cs_etm_pmu, cpu, + metadata_etmv4_ro[CS_ETE_TRCDEVARCH]); + + /* How much space was used */ + increment = CS_ETE_PRIV_MAX; + nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1; + } else if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index f4b2bff533f3..d540512a3c96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2512,6 +2512,7 @@ static const char * const cs_etmv4_priv_fmts[] = { [CS_ETMV4_TRCIDR2] = " TRCIDR2 %llx\n", [CS_ETMV4_TRCIDR8] = " TRCIDR8 %llx\n", [CS_ETMV4_TRCAUTHSTATUS] = " TRCAUTHSTATUS %llx\n", + [CS_ETE_TRCDEVARCH] = " TRCDEVARCH %llx\n" };
static const char * const param_unk_fmt = @@ -2571,10 +2572,15 @@ static int cs_etm__print_cpu_metadata_v1(__u64 *val, int *offset) else fprintf(stdout, cs_etm_priv_fmts[j], val[i]); } - } else if (magic == __perf_cs_etmv4_magic) { + } else if (magic == __perf_cs_etmv4_magic || magic == __perf_cs_ete_magic) { + /* + * ETE and ETMv4 can be printed in the same block because the number of parameters + * is saved and they share the list of parameter names. ETE is also only supported + * in V1 files. + */ for (j = 0; j < total_params; j++, i++) { /* if newer record - could be excess params */ - if (j >= CS_ETMV4_PRIV_MAX) + if (j >= CS_ETE_PRIV_MAX) fprintf(stdout, param_unk_fmt, j, val[i]); else fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]); @@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
/* The traceID is our handle */ trcidr_idx = CS_ETMV4_TRCTRACEIDR; + } else if (ptr[i] == __perf_cs_ete_magic) { + metadata[j] = + cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1); + + /* ETE shares first part of metadata with ETMv4 */ + trcidr_idx = CS_ETMV4_TRCTRACEIDR; }
if (!metadata[j]) { diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index d65c7b19407d..08b84c21f065 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -76,6 +76,14 @@ enum { /* define fixed version 0 length - allow new format reader to read old files. */ #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1)
+/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was + * added in header V1 + */ +enum { + CS_ETE_TRCDEVARCH = CS_ETMV4_PRIV_MAX, + CS_ETE_PRIV_MAX +}; + /* * ETMv3 exception encoding number: * See Embedded Trace Macrocell specification (ARM IHI 0014Q) @@ -187,8 +195,10 @@ struct cs_etm_packet_queue {
#define __perf_cs_etmv3_magic 0x3030303030303030ULL #define __perf_cs_etmv4_magic 0x4040404040404040ULL +#define __perf_cs_ete_magic 0x5050505050505050ULL #define CS_ETMV3_PRIV_SIZE (CS_ETM_PRIV_MAX * sizeof(u64)) #define CS_ETMV4_PRIV_SIZE (CS_ETMV4_PRIV_MAX * sizeof(u64)) +#define CS_ETE_PRIV_SIZE (CS_ETE_PRIV_MAX * sizeof(u64))
#ifdef HAVE_CSTRACE_SUPPORT int cs_etm__process_auxtrace_info(union perf_event *event,
On Fri, Aug 06, 2021 at 02:41:04PM +0100, James Clark wrote:
When ETE is present save the TRCDEVARCH register and set a new magic number. It will be used to configure the decoder in a later commit.
Old versions of perf will not be able to open files with this new magic number, but old files will still work with newer versions of perf.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++---- tools/perf/util/cs-etm.c | 16 ++++++++-- tools/perf/util/cs-etm.h | 10 +++++++ 3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ecb6fa55a210..e3500b79d972 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = { [CS_ETM_ETMIDR] = "mgmt/etmidr", }; -static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = { +static const char * const metadata_etmv4_ro[] = { [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0", [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1", [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2", [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8", [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
- [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch"
}; static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu); +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu); static int cs_etm_set_context_id(struct auxtrace_record *itr, struct evsel *evsel, int cpu) @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, struct evlist *evlist __maybe_unused) { int i;
- int etmv3 = 0, etmv4 = 0;
- int etmv3 = 0, etmv4 = 0, ete = 0; struct perf_cpu_map *event_cpus = evlist->core.cpus; struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
@@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, !cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, if (!cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, perf_cpu_map__put(online_cpus); return (CS_ETM_HEADER_SIZE +
(ete * CS_ETE_PRIV_SIZE) + (etmv4 * CS_ETMV4_PRIV_SIZE) + (etmv3 * CS_ETMV3_PRIV_SIZE));} @@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; } +#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)
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu) +{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_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;
+}
static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) { struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr); @@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; /* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_etmv4(itr, cpu)) {
- if (cs_etm_is_ete(itr, cpu)) {
magic = __perf_cs_ete_magic;/* ETE uses the same registers as ETMv4 plus TRCDEVARCH */cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);info->priv[*offset + CS_ETE_TRCDEVARCH] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);/* How much space was used */increment = CS_ETE_PRIV_MAX;nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;- } else if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index f4b2bff533f3..d540512a3c96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2512,6 +2512,7 @@ static const char * const cs_etmv4_priv_fmts[] = { [CS_ETMV4_TRCIDR2] = " TRCIDR2 %llx\n", [CS_ETMV4_TRCIDR8] = " TRCIDR8 %llx\n", [CS_ETMV4_TRCAUTHSTATUS] = " TRCAUTHSTATUS %llx\n",
- [CS_ETE_TRCDEVARCH] = " TRCDEVARCH %llx\n"
}; static const char * const param_unk_fmt = @@ -2571,10 +2572,15 @@ static int cs_etm__print_cpu_metadata_v1(__u64 *val, int *offset) else fprintf(stdout, cs_etm_priv_fmts[j], val[i]); }
- } else if (magic == __perf_cs_etmv4_magic) {
- } else if (magic == __perf_cs_etmv4_magic || magic == __perf_cs_ete_magic) {
/** ETE and ETMv4 can be printed in the same block because the number of parameters* is saved and they share the list of parameter names. ETE is also only supported* in V1 files.*/
if (j >= CS_ETMV4_PRIV_MAX)
if (j >= CS_ETE_PRIV_MAX) fprintf(stdout, param_unk_fmt, j, val[i]); else fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);@@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* The traceID is our handle */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else if (ptr[i] == __perf_cs_ete_magic) {metadata[j] =cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);
The last parameter is passed with '-1', means the ETE metadata block uses version 1, nr_params_v0 is not used thus pass '-1' for it.
It is fine for me.
/* ETE shares first part of metadata with ETMv4 */trcidr_idx = CS_ETMV4_TRCTRACEIDR;if (!metadata[j]) { diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index d65c7b19407d..08b84c21f065 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -76,6 +76,14 @@ enum { /* define fixed version 0 length - allow new format reader to read old files. */ #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1) +/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was
- added in header V1
- */
+enum {
- CS_ETE_TRCDEVARCH = CS_ETMV4_PRIV_MAX,
- CS_ETE_PRIV_MAX
+};
/*
- ETMv3 exception encoding number:
- See Embedded Trace Macrocell specification (ARM IHI 0014Q)
@@ -187,8 +195,10 @@ struct cs_etm_packet_queue { #define __perf_cs_etmv3_magic 0x3030303030303030ULL #define __perf_cs_etmv4_magic 0x4040404040404040ULL +#define __perf_cs_ete_magic 0x5050505050505050ULL #define CS_ETMV3_PRIV_SIZE (CS_ETM_PRIV_MAX * sizeof(u64)) #define CS_ETMV4_PRIV_SIZE (CS_ETMV4_PRIV_MAX * sizeof(u64)) +#define CS_ETE_PRIV_SIZE (CS_ETE_PRIV_MAX * sizeof(u64))
I went through the patch twice and looks good to me:
Reviewed-by: Leo Yan leo.yan@linaro.org
#ifdef HAVE_CSTRACE_SUPPORT int cs_etm__process_auxtrace_info(union perf_event *event, -- 2.28.0
Hi James
On 06/08/2021 14:41, James Clark wrote:
When ETE is present save the TRCDEVARCH register and set a new magic number. It will be used to configure the decoder in a later commit.
Old versions of perf will not be able to open files with this new magic number, but old files will still work with newer versions of perf.
The patch looks good to me. There are some minor style related comments below.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++---- tools/perf/util/cs-etm.c | 16 ++++++++-- tools/perf/util/cs-etm.h | 10 +++++++ 3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ecb6fa55a210..e3500b79d972 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = { [CS_ETM_ETMIDR] = "mgmt/etmidr", }; -static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = { +static const char * const metadata_etmv4_ro[] = { [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0", [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1", [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2", [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8", [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
- [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch" };
static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu); +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu); static int cs_etm_set_context_id(struct auxtrace_record *itr, struct evsel *evsel, int cpu) @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, struct evlist *evlist __maybe_unused) { int i;
- int etmv3 = 0, etmv4 = 0;
- int etmv3 = 0, etmv4 = 0, ete = 0; struct perf_cpu_map *event_cpus = evlist->core.cpus; struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
@@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, !cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, if (!cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, perf_cpu_map__put(online_cpus); return (CS_ETM_HEADER_SIZE +
(ete * CS_ETE_PRIV_SIZE) + (etmv4 * CS_ETMV4_PRIV_SIZE) + (etmv3 * CS_ETMV3_PRIV_SIZE));@@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; } +#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)
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu) +{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_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;
+}
- static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) { struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
@@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; /* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_etmv4(itr, cpu)) {
- if (cs_etm_is_ete(itr, cpu)) {
magic = __perf_cs_ete_magic;/* ETE uses the same registers as ETMv4 plus TRCDEVARCH */cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);info->priv[*offset + CS_ETE_TRCDEVARCH] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);
minor nit: It may be nicer to have a helper function like we do for etmv4. i.e, cs_etm_save_ete_header();
to keep the code consistent.
/* How much space was used */increment = CS_ETE_PRIV_MAX;nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;- } else if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index f4b2bff533f3..d540512a3c96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c
for (j = 0; j < total_params; j++, i++) { /* if newer record - could be excess params */
if (j >= CS_ETMV4_PRIV_MAX)
if (j >= CS_ETE_PRIV_MAX) fprintf(stdout, param_unk_fmt, j, val[i]); else fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);@@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* The traceID is our handle */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else if (ptr[i] == __perf_cs_ete_magic) {metadata[j] =cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);
minor nit: I would prefer the arguments to be split into the next line, rather than moving the function call to the next line. i.e,
metatdata[j] = cs_etm_create_meta_blk(ptr, &i, ....
/* ETE shares first part of metadata with ETMv4 */trcidr_idx = CS_ETMV4_TRCTRACEIDR;if (!metadata[j]) { diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index d65c7b19407d..08b84c21f065 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -76,6 +76,14 @@ enum { /* define fixed version 0 length - allow new format reader to read old files. */ #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1) +/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was
minor nit: comment style.
/* * ETE metadata... * */
Suzuki
Em Fri, Sep 03, 2021 at 10:08:22AM +0100, Suzuki K Poulose escreveu:
Hi James
On 06/08/2021 14:41, James Clark wrote:
When ETE is present save the TRCDEVARCH register and set a new magic number. It will be used to configure the decoder in a later commit.
Old versions of perf will not be able to open files with this new magic number, but old files will still work with newer versions of perf.
The patch looks good to me. There are some minor style related comments below.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++---- tools/perf/util/cs-etm.c | 16 ++++++++-- tools/perf/util/cs-etm.h | 10 +++++++ 3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ecb6fa55a210..e3500b79d972 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = { [CS_ETM_ETMIDR] = "mgmt/etmidr", }; -static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = { +static const char * const metadata_etmv4_ro[] = { [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0", [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1", [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2", [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8", [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
- [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch" }; static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu); static int cs_etm_set_context_id(struct auxtrace_record *itr, struct evsel *evsel, int cpu) @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, struct evlist *evlist __maybe_unused) { int i;
- int etmv3 = 0, etmv4 = 0;
- int etmv3 = 0, etmv4 = 0, ete = 0; struct perf_cpu_map *event_cpus = evlist->core.cpus; struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
@@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, !cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, if (!cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, perf_cpu_map__put(online_cpus); return (CS_ETM_HEADER_SIZE +
(ete * CS_ETE_PRIV_SIZE) + (etmv4 * CS_ETMV4_PRIV_SIZE) + (etmv3 * CS_ETMV3_PRIV_SIZE));@@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; } +#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)
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu) +{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_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;
+}
- static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) { struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
@@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; /* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_etmv4(itr, cpu)) {
- if (cs_etm_is_ete(itr, cpu)) {
magic = __perf_cs_ete_magic;/* ETE uses the same registers as ETMv4 plus TRCDEVARCH */cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);info->priv[*offset + CS_ETE_TRCDEVARCH] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);minor nit: It may be nicer to have a helper function like we do for etmv4. i.e, cs_etm_save_ete_header();
to keep the code consistent.
/* How much space was used */increment = CS_ETE_PRIV_MAX;nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;- } else if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index f4b2bff533f3..d540512a3c96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c
for (j = 0; j < total_params; j++, i++) { /* if newer record - could be excess params */
if (j >= CS_ETMV4_PRIV_MAX)
if (j >= CS_ETE_PRIV_MAX) fprintf(stdout, param_unk_fmt, j, val[i]); else fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);@@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* The traceID is our handle */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else if (ptr[i] == __perf_cs_ete_magic) {metadata[j] =cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);minor nit: I would prefer the arguments to be split into the next line, rather than moving the function call to the next line. i.e,
metatdata[j] = cs_etm_create_meta_blk(ptr, &i, ....
He did it for consistency with the previous else blocks, but I think it should even be all in just one line, 92 chars is ok, I'd say.
Here I have:
$ echo $COLUMNS 273 $
:-)
I did it as there are plenty of other lines in this file that are over 100 chars in width.
/* ETE shares first part of metadata with ETMv4 */trcidr_idx = CS_ETMV4_TRCTRACEIDR;diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index d65c7b19407d..08b84c21f065 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -76,6 +76,14 @@ enum { /* define fixed version 0 length - allow new format reader to read old files. */ #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1) +/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was
minor nit: comment style.
/* * ETE metadata... * */
Yeah, being even consistent with other multi line comments on this same file.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 12:15, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 10:08:22AM +0100, Suzuki K Poulose escreveu:
Hi James
On 06/08/2021 14:41, James Clark wrote:
When ETE is present save the TRCDEVARCH register and set a new magic number. It will be used to configure the decoder in a later commit.
Old versions of perf will not be able to open files with this new magic number, but old files will still work with newer versions of perf.
The patch looks good to me. There are some minor style related comments below.
Signed-off-by: James Clark james.clark@arm.com
tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++---- tools/perf/util/cs-etm.c | 16 ++++++++-- tools/perf/util/cs-etm.h | 10 +++++++ 3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ecb6fa55a210..e3500b79d972 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = { [CS_ETM_ETMIDR] = "mgmt/etmidr", }; -static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = { +static const char * const metadata_etmv4_ro[] = { [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0", [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1", [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2", [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8", [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
- [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch" }; static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu); static int cs_etm_set_context_id(struct auxtrace_record *itr, struct evsel *evsel, int cpu) @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, struct evlist *evlist __maybe_unused) { int i;
- int etmv3 = 0, etmv4 = 0;
- int etmv3 = 0, etmv4 = 0, ete = 0; struct perf_cpu_map *event_cpus = evlist->core.cpus; struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
@@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, !cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, if (!cpu_map__has(online_cpus, i)) continue;
if (cs_etm_is_etmv4(itr, i))
if (cs_etm_is_ete(itr, i))ete++;else if (cs_etm_is_etmv4(itr, i)) etmv4++; else etmv3++;@@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused, perf_cpu_map__put(online_cpus); return (CS_ETM_HEADER_SIZE +
(ete * CS_ETE_PRIV_SIZE) + (etmv4 * CS_ETMV4_PRIV_SIZE) + (etmv3 * CS_ETMV3_PRIV_SIZE));@@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path) return val; } +#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)
+static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu) +{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_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;
+}
- static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu) { struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
@@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; /* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_etmv4(itr, cpu)) {
- if (cs_etm_is_ete(itr, cpu)) {
magic = __perf_cs_ete_magic;/* ETE uses the same registers as ETMv4 plus TRCDEVARCH */cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);info->priv[*offset + CS_ETE_TRCDEVARCH] =cs_etm_get_ro(cs_etm_pmu, cpu,metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);minor nit: It may be nicer to have a helper function like we do for etmv4. i.e, cs_etm_save_ete_header();
to keep the code consistent.
/* How much space was used */increment = CS_ETE_PRIV_MAX;nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;- } else if (cs_etm_is_etmv4(itr, cpu)) { magic = __perf_cs_etmv4_magic; cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index f4b2bff533f3..d540512a3c96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c
for (j = 0; j < total_params; j++, i++) { /* if newer record - could be excess params */
if (j >= CS_ETMV4_PRIV_MAX)
if (j >= CS_ETE_PRIV_MAX) fprintf(stdout, param_unk_fmt, j, val[i]); else fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);@@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* The traceID is our handle */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else if (ptr[i] == __perf_cs_ete_magic) {metadata[j] =cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);minor nit: I would prefer the arguments to be split into the next line, rather than moving the function call to the next line. i.e,
metatdata[j] = cs_etm_create_meta_blk(ptr, &i, ....He did it for consistency with the previous else blocks, but I think it should even be all in just one line, 92 chars is ok, I'd say.
Here I have:
$ echo $COLUMNS 273 $
:-)
I did it as there are plenty of other lines in this file that are over 100 chars in width.
/* ETE shares first part of metadata with ETMv4 */trcidr_idx = CS_ETMV4_TRCTRACEIDR; } if (!metadata[j]) {diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index d65c7b19407d..08b84c21f065 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -76,6 +76,14 @@ enum { /* define fixed version 0 length - allow new format reader to read old files. */ #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1) +/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was
minor nit: comment style.
/* * ETE metadata... * */Yeah, being even consistent with other multi line comments on this same file.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
TRCIRD2 should be TRCIDR2
Signed-off-by: James Clark james.clark@arm.com --- tools/perf/arch/arm/util/cs-etm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index e3500b79d972..515aae470e23 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -75,7 +75,7 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr, if (!cs_etm_is_etmv4(itr, cpu)) goto out;
- /* Get a handle on TRCIRD2 */ + /* Get a handle on TRCIDR2 */ snprintf(path, PATH_MAX, "cpu%d/%s", cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2]); err = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
On Fri, Aug 06, 2021 at 02:41:05PM +0100, James Clark wrote:
TRCIRD2 should be TRCIDR2
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
tools/perf/arch/arm/util/cs-etm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index e3500b79d972..515aae470e23 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -75,7 +75,7 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr, if (!cs_etm_is_etmv4(itr, cpu)) goto out;
- /* Get a handle on TRCIRD2 */
- /* Get a handle on TRCIDR2 */ snprintf(path, PATH_MAX, "cpu%d/%s", cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2]); err = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
-- 2.28.0
On 06/08/2021 14:41, James Clark wrote:
TRCIRD2 should be TRCIDR2
Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 10:09, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 06/08/2021 14:41, James Clark wrote:
TRCIRD2 should be TRCIDR2
Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
OpenCSD v1.1.1 has a bug fix for the installation of the ETE decoder headers. This also means that including headers separately for each decoder is unnecessary so remove these.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com --- tools/build/feature/test-libopencsd.c | 4 ++-- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/build/feature/test-libopencsd.c b/tools/build/feature/test-libopencsd.c index 52c790b0317b..eb6303ff446e 100644 --- a/tools/build/feature/test-libopencsd.c +++ b/tools/build/feature/test-libopencsd.c @@ -4,9 +4,9 @@ /* * Check OpenCSD library version is sufficient to provide required features */ -#define OCSD_MIN_VER ((1 << 16) | (0 << 8) | (0)) +#define OCSD_MIN_VER ((1 << 16) | (1 << 8) | (1)) #if !defined(OCSD_VER_NUM) || (OCSD_VER_NUM < OCSD_MIN_VER) -#error "OpenCSD >= 1.0.0 is required" +#error "OpenCSD >= 1.1.1 is required" #endif
int main(void) 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 787b19642e78..12cee321fbf2 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -13,8 +13,6 @@ #include <linux/zalloc.h> #include <stdlib.h> #include <opencsd/c_api/opencsd_c_api.h> -#include <opencsd/etmv4/trc_pkt_types_etmv4.h> -#include <opencsd/ocsd_if_types.h>
#include "cs-etm.h" #include "cs-etm-decoder.h"
On 06/08/2021 14:41, James Clark wrote:
OpenCSD v1.1.1 has a bug fix for the installation of the ETE decoder headers. This also means that including headers separately for each decoder is unnecessary so remove these.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Em Fri, Sep 03, 2021 at 10:10:01AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
OpenCSD v1.1.1 has a bug fix for the installation of the ETE decoder headers. This also means that including headers separately for each decoder is unnecessary so remove these.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 12:16, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 10:10:01AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
OpenCSD v1.1.1 has a bug fix for the installation of the ETE decoder headers. This also means that including headers separately for each decoder is unnecessary so remove these.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
If the magic number indicates ETE instantiate a 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 and v1 headers for ETMv4 still open correctly * New files with new magic number open on new versions of perf * New files with new magic number fail to open on old versions of perf * Decoding with the ETE decoder results in the same output as the ETMv4 decoder as long as 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 | 20 +++++++++++++++++++ .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 12 +++++++++++ tools/perf/util/cs-etm.c | 18 +++++++++++++++++ 3 files changed, 50 insertions(+)
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 12cee321fbf2..3071e5deddcc 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -156,6 +156,20 @@ static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, config->core_prof = profile_CortexA; }
+static void cs_etm_decoder__gen_ete_config(struct cs_etm_trace_params *params, + ocsd_ete_cfg *config) +{ + config->reg_configr = params->ete.reg_configr; + config->reg_traceidr = params->ete.reg_traceidr; + config->reg_idr0 = params->ete.reg_idr0; + config->reg_idr1 = params->ete.reg_idr1; + config->reg_idr2 = params->ete.reg_idr2; + config->reg_idr8 = params->ete.reg_idr8; + config->reg_devarch = params->ete.reg_devarch; + config->arch_ver = ARCH_AA64; + config->core_prof = profile_CortexA; +} + static void cs_etm_decoder__print_str_cb(const void *p_context, const char *msg, const int str_len) @@ -603,6 +617,7 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, const char *decoder_name; ocsd_etmv3_cfg config_etmv3; ocsd_etmv4_cfg trace_config_etmv4; + ocsd_ete_cfg trace_config_ete; void *trace_config; u8 csid;
@@ -620,6 +635,11 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, decoder_name = OCSD_BUILTIN_DCD_ETMV4I; trace_config = &trace_config_etmv4; break; + case CS_ETM_PROTO_ETE: + cs_etm_decoder__gen_ete_config(t_params, &trace_config_ete); + decoder_name = OCSD_BUILTIN_DCD_ETE; + trace_config = &trace_config_ete; + break; default: return -1; } 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..0102ece5ca3e 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,22 @@ struct cs_etmv4_trace_params { u32 reg_traceidr; };
+struct cs_ete_trace_params { + u32 reg_idr0; + u32 reg_idr1; + u32 reg_idr2; + u32 reg_idr8; + u32 reg_configr; + u32 reg_traceidr; + u32 reg_devarch; +}; + struct cs_etm_trace_params { int protocol; union { struct cs_etmv3_trace_params etmv3; struct cs_etmv4_trace_params etmv4; + struct cs_ete_trace_params ete; }; };
@@ -65,6 +76,7 @@ enum { CS_ETM_PROTO_ETMV4i, CS_ETM_PROTO_ETMV4d, CS_ETM_PROTO_PTM, + CS_ETM_PROTO_ETE };
enum cs_etm_decoder_operation { diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index d540512a3c96..e5649e9ea140 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -460,6 +460,21 @@ static void cs_etm__set_trace_param_etmv4(struct cs_etm_trace_params *t_params, t_params[idx].etmv4.reg_traceidr = metadata[idx][CS_ETMV4_TRCTRACEIDR]; }
+static void cs_etm__set_trace_param_ete(struct cs_etm_trace_params *t_params, + struct cs_etm_auxtrace *etm, int idx) +{ + u64 **metadata = etm->metadata; + + t_params[idx].protocol = CS_ETM_PROTO_ETE; + t_params[idx].ete.reg_idr0 = metadata[idx][CS_ETMV4_TRCIDR0]; + t_params[idx].ete.reg_idr1 = metadata[idx][CS_ETMV4_TRCIDR1]; + t_params[idx].ete.reg_idr2 = metadata[idx][CS_ETMV4_TRCIDR2]; + t_params[idx].ete.reg_idr8 = metadata[idx][CS_ETMV4_TRCIDR8]; + t_params[idx].ete.reg_configr = metadata[idx][CS_ETMV4_TRCCONFIGR]; + t_params[idx].ete.reg_traceidr = metadata[idx][CS_ETMV4_TRCTRACEIDR]; + t_params[idx].ete.reg_devarch = metadata[idx][CS_ETE_TRCDEVARCH]; +} + static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, struct cs_etm_auxtrace *etm, int decoders) @@ -479,6 +494,9 @@ static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, case __perf_cs_etmv4_magic: cs_etm__set_trace_param_etmv4(t_params, etm, i); break; + case __perf_cs_ete_magic: + cs_etm__set_trace_param_ete(t_params, etm, i); + break; default: return -EINVAL; }
On Fri, Aug 06, 2021 at 02:41:07PM +0100, James Clark wrote:
If the magic number indicates ETE instantiate a 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 and v1 headers for ETMv4 still open correctly
- New files with new magic number open on new versions of perf
- New files with new magic number fail to open on old versions of perf
- Decoding with the ETE decoder results in the same output as the ETMv4 decoder as long as 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 | 20 +++++++++++++++++++ .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 12 +++++++++++ tools/perf/util/cs-etm.c | 18 +++++++++++++++++ 3 files changed, 50 insertions(+)
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 12cee321fbf2..3071e5deddcc 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -156,6 +156,20 @@ static void cs_etm_decoder__gen_etmv4_config(struct cs_etm_trace_params *params, config->core_prof = profile_CortexA; } +static void cs_etm_decoder__gen_ete_config(struct cs_etm_trace_params *params,
ocsd_ete_cfg *config)+{
- config->reg_configr = params->ete.reg_configr;
- config->reg_traceidr = params->ete.reg_traceidr;
- config->reg_idr0 = params->ete.reg_idr0;
- config->reg_idr1 = params->ete.reg_idr1;
- config->reg_idr2 = params->ete.reg_idr2;
- config->reg_idr8 = params->ete.reg_idr8;
- config->reg_devarch = params->ete.reg_devarch;
- config->arch_ver = ARCH_AA64;
Just a nitpick: if we connect with patch 02, it implement function cs_etm_decoder__get_etmv4_arch_ver(). We can extend it to a more general function cs_etm_decoder__get_arch_ver(); this can allow us to have a central place to calculate 'arch_ver' for different archs.
I understand your patch doesn't want to mix things between ETMv4 and ETE, either way is okay for me:
Reviewed-by: Leo Yan leo.yan@linaro.org
- config->core_prof = profile_CortexA;
+}
static void cs_etm_decoder__print_str_cb(const void *p_context, const char *msg, const int str_len) @@ -603,6 +617,7 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, const char *decoder_name; ocsd_etmv3_cfg config_etmv3; ocsd_etmv4_cfg trace_config_etmv4;
- ocsd_ete_cfg trace_config_ete; void *trace_config; u8 csid;
@@ -620,6 +635,11 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, decoder_name = OCSD_BUILTIN_DCD_ETMV4I; trace_config = &trace_config_etmv4; break;
- case CS_ETM_PROTO_ETE:
cs_etm_decoder__gen_ete_config(t_params, &trace_config_ete);decoder_name = OCSD_BUILTIN_DCD_ETE;trace_config = &trace_config_ete;break;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..0102ece5ca3e 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,22 @@ struct cs_etmv4_trace_params { u32 reg_traceidr; }; +struct cs_ete_trace_params {
- u32 reg_idr0;
- u32 reg_idr1;
- u32 reg_idr2;
- u32 reg_idr8;
- u32 reg_configr;
- u32 reg_traceidr;
- u32 reg_devarch;
+};
struct cs_etm_trace_params { int protocol; union { struct cs_etmv3_trace_params etmv3; struct cs_etmv4_trace_params etmv4;
struct cs_ete_trace_params ete;}; @@ -65,6 +76,7 @@ enum { CS_ETM_PROTO_ETMV4i, CS_ETM_PROTO_ETMV4d, CS_ETM_PROTO_PTM,
- CS_ETM_PROTO_ETE
}; enum cs_etm_decoder_operation { diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index d540512a3c96..e5649e9ea140 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -460,6 +460,21 @@ static void cs_etm__set_trace_param_etmv4(struct cs_etm_trace_params *t_params, t_params[idx].etmv4.reg_traceidr = metadata[idx][CS_ETMV4_TRCTRACEIDR]; } +static void cs_etm__set_trace_param_ete(struct cs_etm_trace_params *t_params,
struct cs_etm_auxtrace *etm, int idx)+{
- u64 **metadata = etm->metadata;
- t_params[idx].protocol = CS_ETM_PROTO_ETE;
- t_params[idx].ete.reg_idr0 = metadata[idx][CS_ETMV4_TRCIDR0];
- t_params[idx].ete.reg_idr1 = metadata[idx][CS_ETMV4_TRCIDR1];
- t_params[idx].ete.reg_idr2 = metadata[idx][CS_ETMV4_TRCIDR2];
- t_params[idx].ete.reg_idr8 = metadata[idx][CS_ETMV4_TRCIDR8];
- t_params[idx].ete.reg_configr = metadata[idx][CS_ETMV4_TRCCONFIGR];
- t_params[idx].ete.reg_traceidr = metadata[idx][CS_ETMV4_TRCTRACEIDR];
- t_params[idx].ete.reg_devarch = metadata[idx][CS_ETE_TRCDEVARCH];
+}
static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, struct cs_etm_auxtrace *etm, int decoders) @@ -479,6 +494,9 @@ static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, case __perf_cs_etmv4_magic: cs_etm__set_trace_param_etmv4(t_params, etm, i); break;
case __perf_cs_ete_magic:cs_etm__set_trace_param_ete(t_params, etm, i);break;-- 2.28.0
On 24/08/2021 09:33, Leo Yan wrote:
+static void cs_etm_decoder__gen_ete_config(struct cs_etm_trace_params *params,
ocsd_ete_cfg *config)+{
- config->reg_configr = params->ete.reg_configr;
- config->reg_traceidr = params->ete.reg_traceidr;
- config->reg_idr0 = params->ete.reg_idr0;
- config->reg_idr1 = params->ete.reg_idr1;
- config->reg_idr2 = params->ete.reg_idr2;
- config->reg_idr8 = params->ete.reg_idr8;
- config->reg_devarch = params->ete.reg_devarch;
- config->arch_ver = ARCH_AA64;
Just a nitpick: if we connect with patch 02, it implement function cs_etm_decoder__get_etmv4_arch_ver(). We can extend it to a more general function cs_etm_decoder__get_arch_ver(); this can allow us to have a central place to calculate 'arch_ver' for different archs.
I understand your patch doesn't want to mix things between ETMv4 and ETE, either way is okay for me:
Hi Leo,
Yes that was one of the previous comments from Mike that the logic only applied to ETMv4 so I made it v4 specific. The other arch_vers are fixed at the moment. If we get a new one that needs changing depending on some config we can try to add a generic function.
Reviewed-by: Leo Yan leo.yan@linaro.org
On 06/08/2021 14:41, James Clark wrote:
If the magic number indicates ETE instantiate a 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 and v1 headers for ETMv4 still open correctly
- New files with new magic number open on new versions of perf
- New files with new magic number fail to open on old versions of perf
- Decoding with the ETE decoder results in the same output as the ETMv4 decoder as long as there are no new ETE packet types
Signed-off-by: James Clark james.clark@arm.com
Changes look good to me with my limited knowledge about the decoder facing code.
FWIW,
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Em Fri, Sep 03, 2021 at 10:15:07AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
If the magic number indicates ETE instantiate a 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 and v1 headers for ETMv4 still open correctly
- New files with new magic number open on new versions of perf
- New files with new magic number fail to open on old versions of perf
- Decoding with the ETE decoder results in the same output as the ETMv4 decoder as long as there are no new ETE packet types
Signed-off-by: James Clark james.clark@arm.com
Changes look good to me with my limited knowledge about the decoder facing code.
FWIW,
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 12:16, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 10:15:07AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
If the magic number indicates ETE instantiate a 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 and v1 headers for ETMv4 still open correctly
- New files with new magic number open on new versions of perf
- New files with new magic number fail to open on old versions of perf
- Decoding with the ETE decoder results in the same output as the ETMv4 decoder as long as there are no new ETE packet types
Signed-off-by: James Clark james.clark@arm.com
Changes look good to me with my limited knowledge about the decoder facing code.
FWIW,
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Acked-by.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Use the real name of the decoder instead of hard-coding "ETM" to avoid confusion when the trace is ETE. This also now distinguishes between ETMv3 and ETMv4.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 17 +++++++++++------ tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 1 + tools/perf/util/cs-etm.c | 4 ++-- 3 files changed, 14 insertions(+), 8 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 3071e5deddcc..29a23fa16aff 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -37,6 +37,7 @@ struct cs_etm_decoder { dcd_tree_handle_t dcd_tree; cs_etm_mem_cb_type mem_access; ocsd_datapath_resp_t prev_return; + const char *decoder_name; };
static u32 @@ -614,7 +615,6 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, struct cs_etm_trace_params *t_params, struct cs_etm_decoder *decoder) { - const char *decoder_name; ocsd_etmv3_cfg config_etmv3; ocsd_etmv4_cfg trace_config_etmv4; ocsd_ete_cfg trace_config_ete; @@ -625,19 +625,19 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, case CS_ETM_PROTO_ETMV3: case CS_ETM_PROTO_PTM: cs_etm_decoder__gen_etmv3_config(t_params, &config_etmv3); - decoder_name = (t_params->protocol == CS_ETM_PROTO_ETMV3) ? + decoder->decoder_name = (t_params->protocol == CS_ETM_PROTO_ETMV3) ? OCSD_BUILTIN_DCD_ETMV3 : OCSD_BUILTIN_DCD_PTM; trace_config = &config_etmv3; break; case CS_ETM_PROTO_ETMV4i: cs_etm_decoder__gen_etmv4_config(t_params, &trace_config_etmv4); - decoder_name = OCSD_BUILTIN_DCD_ETMV4I; + decoder->decoder_name = OCSD_BUILTIN_DCD_ETMV4I; trace_config = &trace_config_etmv4; break; case CS_ETM_PROTO_ETE: cs_etm_decoder__gen_ete_config(t_params, &trace_config_ete); - decoder_name = OCSD_BUILTIN_DCD_ETE; + decoder->decoder_name = OCSD_BUILTIN_DCD_ETE; trace_config = &trace_config_ete; break; default: @@ -646,7 +646,7 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params,
if (d_params->operation == CS_ETM_OPERATION_DECODE) { if (ocsd_dt_create_decoder(decoder->dcd_tree, - decoder_name, + decoder->decoder_name, OCSD_CREATE_FLG_FULL_DECODER, trace_config, &csid)) return -1; @@ -658,7 +658,7 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params,
return 0; } else if (d_params->operation == CS_ETM_OPERATION_PRINT) { - if (ocsd_dt_create_decoder(decoder->dcd_tree, decoder_name, + if (ocsd_dt_create_decoder(decoder->dcd_tree, decoder->decoder_name, OCSD_CREATE_FLG_PACKET_PROC, trace_config, &csid)) return -1; @@ -790,3 +790,8 @@ void cs_etm_decoder__free(struct cs_etm_decoder *decoder) decoder->dcd_tree = NULL; free(decoder); } + +const char *cs_etm_decoder__get_name(struct cs_etm_decoder *decoder) +{ + return decoder->decoder_name; +} 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 0102ece5ca3e..92a855fbe5b8 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h @@ -104,5 +104,6 @@ int cs_etm_decoder__get_packet(struct cs_etm_packet_queue *packet_queue, struct cs_etm_packet *packet);
int cs_etm_decoder__reset(struct cs_etm_decoder *decoder); +const char *cs_etm_decoder__get_name(struct cs_etm_decoder *decoder);
#endif /* INCLUDE__CS_ETM_DECODER_H__ */ diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index e5649e9ea140..788ad5a099f6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -537,8 +537,8 @@ static void cs_etm__dump_event(struct cs_etm_queue *etmq,
fprintf(stdout, "\n"); color_fprintf(stdout, color, - ". ... CoreSight ETM Trace data: size %zu bytes\n", - buffer->size); + ". ... CoreSight %s Trace data: size %zu bytes\n", + cs_etm_decoder__get_name(etmq->decoder), buffer->size);
do { size_t consumed;
On 06/08/2021 14:41, James Clark wrote:
Use the real name of the decoder instead of hard-coding "ETM" to avoid confusion when the trace is ETE. This also now distinguishes between ETMv3 and ETMv4.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Em Fri, Sep 03, 2021 at 10:17:25AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
Use the real name of the decoder instead of hard-coding "ETM" to avoid confusion when the trace is ETE. This also now distinguishes between ETMv3 and ETMv4.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Reviewed-by.
- Arnaldo
Em Fri, Sep 03, 2021 at 08:18:46AM -0300, Arnaldo Carvalho de Melo escreveu:
Em Fri, Sep 03, 2021 at 10:17:25AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
Use the real name of the decoder instead of hard-coding "ETM" to avoid confusion when the trace is ETE. This also now distinguishes between ETMv3 and ETMv4.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Reviewed-by.
Everything is in my tmp.perf/core branch now, pending test build in my containers.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Fri, 3 Sept 2021 at 12:52, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Fri, Sep 03, 2021 at 08:18:46AM -0300, Arnaldo Carvalho de Melo escreveu:
Em Fri, Sep 03, 2021 at 10:17:25AM +0100, Suzuki K Poulose escreveu:
On 06/08/2021 14:41, James Clark wrote:
Use the real name of the decoder instead of hard-coding "ETM" to avoid confusion when the trace is ETE. This also now distinguishes between ETMv3 and ETMv4.
Reviewed-by: Leo Yan leo.yan@linaro.org Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Thanks, collected the Reviewed-by.
Everything is in my tmp.perf/core branch now, pending test build in my containers.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com --- tools/perf/util/cs-etm.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 788ad5a099f6..5b276bdb96a6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2973,6 +2973,11 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
/* ETE shares first part of metadata with ETMv4 */ trcidr_idx = CS_ETMV4_TRCTRACEIDR; + } else { + ui__error("CS ETM Trace: Unrecognised magic number %#"PRIx64". File could be from a newer version of perf.\n", + ptr[i]); + err = -EINVAL; + goto err_free_metadata; }
if (!metadata[j]) {
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
tools/perf/util/cs-etm.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 788ad5a099f6..5b276bdb96a6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2973,6 +2973,11 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* ETE shares first part of metadata with ETMv4 */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else {ui__error("CS ETM Trace: Unrecognised magic number %#"PRIx64". File could be from a newer version of perf.\n",ptr[i]);err = -EINVAL;goto err_free_metadata;if (!metadata[j]) { -- 2.28.0
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
- Arnaldo
tools/perf/util/cs-etm.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 788ad5a099f6..5b276bdb96a6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2973,6 +2973,11 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* ETE shares first part of metadata with ETMv4 */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else {ui__error("CS ETM Trace: Unrecognised magic number %#"PRIx64". File could be from a newer version of perf.\n",ptr[i]);err = -EINVAL;goto err_free_metadata;if (!metadata[j]) { -- 2.28.0
Em Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
I´m now trying to fix this up, I applied it using 'b4', so no patch should have gone missing...
⬢[acme@toolbox perf]$ time make -C tools/perf build-test make: Entering directory '/var/home/acme/git/perf/tools/perf' - tarpkg: ./tests/perf-targz-src-pkg . make_static: cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 -j24 DESTDIR=/tmp/tmp.tw23W3JC1W make_with_gtk2: cd . && make GTK2=1 -j24 DESTDIR=/tmp/tmp.F7gN4e98pK make_tags_O: cd . && make tags FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.tQVbhFdXKU DESTDIR=/tmp/tmp.1vbvWgUYUv make_no_slang_O: cd . && make NO_SLANG=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.2L0POmKIip DESTDIR=/tmp/tmp.0qTYEQTY8e make_no_demangle_O: cd . && make NO_DEMANGLE=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.Wh3kRYOFJo DESTDIR=/tmp/tmp.ih1nESGU6N make_no_sdt_O: cd . && make NO_SDT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.zw3NugHqvZ DESTDIR=/tmp/tmp.li1bxbfYOZ make_no_backtrace_O: cd . && make NO_BACKTRACE=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.66VxfiD04f DESTDIR=/tmp/tmp.PIgwBwGEZz make_install_prefix_O: cd . && make install prefix=/tmp/krava FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.s6u85zKKjU DESTDIR=/tmp/tmp.2FJoF1mCRB make_with_coresight_O: cd . && make CORESIGHT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.kQs4YxWFpL DESTDIR=/tmp/tmp.z93leAThcc cd . && make CORESIGHT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.kQs4YxWFpL DESTDIR=/tmp/tmp.z93leAThcc BUILD: Doing 'make -j24' parallel build HOSTCC /tmp/tmp.kQs4YxWFpL/fixdep.o HOSTLD /tmp/tmp.kQs4YxWFpL/fixdep-in.o LINK /tmp/tmp.kQs4YxWFpL/fixdep Makefile.config:1038: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel GEN /tmp/tmp.kQs4YxWFpL/common-cmds.h CC /tmp/tmp.kQs4YxWFpL/exec-cmd.o CC /tmp/tmp.kQs4YxWFpL/help.o <SNIP> CC /tmp/tmp.kQs4YxWFpL/util/auxtrace.o MKDIR /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-pkt-decoder.o MKDIR /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/ MKDIR /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/arm-spe-pkt-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/arm-spe-decoder.o MKDIR /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/ GEN /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/inat-tables.c MKDIR /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/cs-etm-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-log.o MKDIR /tmp/tmp.kQs4YxWFpL/util/scripting-engines/ CC /tmp/tmp.kQs4YxWFpL/util/scripting-engines/trace-event-perl.o CC /tmp/tmp.kQs4YxWFpL/util/intel-bts.o MKDIR /tmp/tmp.kQs4YxWFpL/util/scripting-engines/ CC /tmp/tmp.kQs4YxWFpL/util/intel-pt.o CC /tmp/tmp.kQs4YxWFpL/util/scripting-engines/trace-event-python.o CC /tmp/tmp.kQs4YxWFpL/util/arm-spe.o CC /tmp/tmp.kQs4YxWFpL/util/s390-cpumsf.o util/cs-etm-decoder/cs-etm-decoder.c:161:44: error: unknown type name ‘ocsd_ete_cfg’; did you mean ‘ocsd_stm_cfg’? 161 | ocsd_ete_cfg *config) | ^~~~~~~~~~~~ | ocsd_stm_cfg util/cs-etm-decoder/cs-etm-decoder.c: In function ‘cs_etm_decoder__create_etm_decoder’: util/cs-etm-decoder/cs-etm-decoder.c:620:9: error: unknown type name ‘ocsd_ete_cfg’; did you mean ‘ocsd_stm_cfg’? 620 | ocsd_ete_cfg trace_config_ete; | ^~~~~~~~~~~~ | ocsd_stm_cfg util/cs-etm-decoder/cs-etm-decoder.c:639:17: error: implicit declaration of function ‘cs_etm_decoder__gen_ete_config’; did you mean ‘cs_etm_decoder__gen_etmv4_config’? [-Werror=implicit-function-declaration] 639 | cs_etm_decoder__gen_ete_config(t_params, &trace_config_ete); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | cs_etm_decoder__gen_etmv4_config cc1: all warnings being treated as errors make[7]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:97: /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/cs-etm-decoder.o] Error 1 make[6]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:139: cs-etm-decoder] Error 2 make[6]: *** Waiting for unfinished jobs.... CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-insn-decoder.o LD /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/perf-in.o LD /tmp/tmp.kQs4YxWFpL/util/scripting-engines/perf-in.o LD /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/perf-in.o make[5]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:139: util] Error 2 make[4]: *** [Makefile.perf:658: /tmp/tmp.kQs4YxWFpL/perf-in.o] Error 2 rm /tmp/tmp.kQs4YxWFpL/dlfilters/dlfilter-test-api-v0.o make[3]: *** [Makefile.perf:238: sub-make] Error 2 make[2]: *** [Makefile:70: all] Error 2 make[1]: *** [tests/make:337: make_with_coresight_O] Error 1 make: *** [Makefile:103: build-test] Error 2 make: Leaving directory '/var/home/acme/git/perf/tools/perf'
real 1m23.257s user 13m37.871s sys 2m53.438s ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$
Em Wed, Sep 01, 2021 at 01:07:41PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
I´m now trying to fix this up, I applied it using 'b4', so no patch should have gone missing...
So its probably related to:
⬢[acme@toolbox perf]$ rpm -qa | grep opencsd opencsd-1.0.0-1.fc34.x86_64 opencsd-devel-1.0.0-1.fc34.x86_64 ⬢[acme@toolbox perf]$
In which case the usual mechanism is to test if we have what is needed via tools/build/feature/test-_____.c, lemme check...
- Arnaldo
⬢[acme@toolbox perf]$ time make -C tools/perf build-test make: Entering directory '/var/home/acme/git/perf/tools/perf'
- tarpkg: ./tests/perf-targz-src-pkg . make_static: cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 -j24 DESTDIR=/tmp/tmp.tw23W3JC1W make_with_gtk2: cd . && make GTK2=1 -j24 DESTDIR=/tmp/tmp.F7gN4e98pK make_tags_O: cd . && make tags FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.tQVbhFdXKU DESTDIR=/tmp/tmp.1vbvWgUYUv make_no_slang_O: cd . && make NO_SLANG=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.2L0POmKIip DESTDIR=/tmp/tmp.0qTYEQTY8e make_no_demangle_O: cd . && make NO_DEMANGLE=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.Wh3kRYOFJo DESTDIR=/tmp/tmp.ih1nESGU6N make_no_sdt_O: cd . && make NO_SDT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.zw3NugHqvZ DESTDIR=/tmp/tmp.li1bxbfYOZ make_no_backtrace_O: cd . && make NO_BACKTRACE=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.66VxfiD04f DESTDIR=/tmp/tmp.PIgwBwGEZz make_install_prefix_O: cd . && make install prefix=/tmp/krava FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.s6u85zKKjU DESTDIR=/tmp/tmp.2FJoF1mCRB make_with_coresight_O: cd . && make CORESIGHT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.kQs4YxWFpL DESTDIR=/tmp/tmp.z93leAThcc
cd . && make CORESIGHT=1 FEATURES_DUMP=/var/home/acme/git/perf/tools/perf/BUILD_TEST_FEATURE_DUMP -j24 O=/tmp/tmp.kQs4YxWFpL DESTDIR=/tmp/tmp.z93leAThcc BUILD: Doing 'make -j24' parallel build HOSTCC /tmp/tmp.kQs4YxWFpL/fixdep.o HOSTLD /tmp/tmp.kQs4YxWFpL/fixdep-in.o LINK /tmp/tmp.kQs4YxWFpL/fixdep Makefile.config:1038: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel GEN /tmp/tmp.kQs4YxWFpL/common-cmds.h CC /tmp/tmp.kQs4YxWFpL/exec-cmd.o CC /tmp/tmp.kQs4YxWFpL/help.o
<SNIP> CC /tmp/tmp.kQs4YxWFpL/util/auxtrace.o MKDIR /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-pkt-decoder.o MKDIR /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/ MKDIR /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/arm-spe-pkt-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/arm-spe-decoder.o MKDIR /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/ GEN /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/inat-tables.c MKDIR /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/ CC /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/cs-etm-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-log.o MKDIR /tmp/tmp.kQs4YxWFpL/util/scripting-engines/ CC /tmp/tmp.kQs4YxWFpL/util/scripting-engines/trace-event-perl.o CC /tmp/tmp.kQs4YxWFpL/util/intel-bts.o MKDIR /tmp/tmp.kQs4YxWFpL/util/scripting-engines/ CC /tmp/tmp.kQs4YxWFpL/util/intel-pt.o CC /tmp/tmp.kQs4YxWFpL/util/scripting-engines/trace-event-python.o CC /tmp/tmp.kQs4YxWFpL/util/arm-spe.o CC /tmp/tmp.kQs4YxWFpL/util/s390-cpumsf.o util/cs-etm-decoder/cs-etm-decoder.c:161:44: error: unknown type name ‘ocsd_ete_cfg’; did you mean ‘ocsd_stm_cfg’? 161 | ocsd_ete_cfg *config) | ^~~~~~~~~~~~ | ocsd_stm_cfg util/cs-etm-decoder/cs-etm-decoder.c: In function ‘cs_etm_decoder__create_etm_decoder’: util/cs-etm-decoder/cs-etm-decoder.c:620:9: error: unknown type name ‘ocsd_ete_cfg’; did you mean ‘ocsd_stm_cfg’? 620 | ocsd_ete_cfg trace_config_ete; | ^~~~~~~~~~~~ | ocsd_stm_cfg util/cs-etm-decoder/cs-etm-decoder.c:639:17: error: implicit declaration of function ‘cs_etm_decoder__gen_ete_config’; did you mean ‘cs_etm_decoder__gen_etmv4_config’? [-Werror=implicit-function-declaration] 639 | cs_etm_decoder__gen_ete_config(t_params, &trace_config_ete); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | cs_etm_decoder__gen_etmv4_config cc1: all warnings being treated as errors make[7]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:97: /tmp/tmp.kQs4YxWFpL/util/cs-etm-decoder/cs-etm-decoder.o] Error 1 make[6]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:139: cs-etm-decoder] Error 2 make[6]: *** Waiting for unfinished jobs.... CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-decoder.o CC /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/intel-pt-insn-decoder.o LD /tmp/tmp.kQs4YxWFpL/util/arm-spe-decoder/perf-in.o LD /tmp/tmp.kQs4YxWFpL/util/scripting-engines/perf-in.o LD /tmp/tmp.kQs4YxWFpL/util/intel-pt-decoder/perf-in.o make[5]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:139: util] Error 2 make[4]: *** [Makefile.perf:658: /tmp/tmp.kQs4YxWFpL/perf-in.o] Error 2 rm /tmp/tmp.kQs4YxWFpL/dlfilters/dlfilter-test-api-v0.o make[3]: *** [Makefile.perf:238: sub-make] Error 2 make[2]: *** [Makefile:70: all] Error 2 make[1]: *** [tests/make:337: make_with_coresight_O] Error 1 make: *** [Makefile:103: build-test] Error 2 make: Leaving directory '/var/home/acme/git/perf/tools/perf'
real 1m23.257s user 13m37.871s sys 2m53.438s ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$
Em Wed, Sep 01, 2021 at 01:16:56PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 01:07:41PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
I´m now trying to fix this up, I applied it using 'b4', so no patch should have gone missing...
So its probably related to:
⬢[acme@toolbox perf]$ rpm -qa | grep opencsd opencsd-1.0.0-1.fc34.x86_64 opencsd-devel-1.0.0-1.fc34.x86_64 ⬢[acme@toolbox perf]$
In which case the usual mechanism is to test if we have what is needed via tools/build/feature/test-_____.c, lemme check...
There is a test and it fails, of course:
⬢[acme@toolbox perf]$ cat /tmp/build/perf/feature/test-libopencsd.make.output test-libopencsd.c:9:2: error: #error "OpenCSD >= 1.1.1 is required" 9 | #error "OpenCSD >= 1.1.1 is required" | ^~~~~ ⬢[acme@toolbox perf]$
But the fact that I ask for CORESIGHT=1 should have the build fail then, i.e. if one explicitely asks for a feature and it can't be built, fail the whole build.
- Arnaldo
Em Wed, Sep 01, 2021 at 01:25:37PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 01:16:56PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 01:07:41PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo escreveu:
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
I´m now trying to fix this up, I applied it using 'b4', so no patch should have gone missing...
So its probably related to:
⬢[acme@toolbox perf]$ rpm -qa | grep opencsd opencsd-1.0.0-1.fc34.x86_64 opencsd-devel-1.0.0-1.fc34.x86_64 ⬢[acme@toolbox perf]$
In which case the usual mechanism is to test if we have what is needed via tools/build/feature/test-_____.c, lemme check...
There is a test and it fails, of course:
⬢[acme@toolbox perf]$ cat /tmp/build/perf/feature/test-libopencsd.make.output test-libopencsd.c:9:2: error: #error "OpenCSD >= 1.1.1 is required" 9 | #error "OpenCSD >= 1.1.1 is required" | ^~~~~ ⬢[acme@toolbox perf]$
But the fact that I ask for CORESIGHT=1 should have the build fail then, i.e. if one explicitely asks for a feature and it can't be built, fail the whole build.
So after uninstalling the libopencsd that comes with fedora 34 and cloning the upstream OpenCSD git repo, building it and installing in /usr/local/ it seems to work as expected:
⬢[acme@toolbox perf]$ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; ⬢[acme@toolbox perf]$ make O=/tmp/build/perf VF=1 CORESIGHT=1 O=/tmp/build/perf -C tools/perf install-bin |& grep -i opencsd ... libopencsd: [ on ] ⬢[acme@toolbox perf]$ cat /tmp/build/perf/feature/test-libopencsd.make.output ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$ ldd ~/bin/perf | grep opencsd libopencsd_c_api.so.1 => not found ⬢[acme@toolbox perf]$ export LD_LIBRARY_PATH=/usr/local/lib ⬢[acme@toolbox perf]$ ldd ~/bin/perf | grep opencsd libopencsd_c_api.so.1 => /usr/local/lib/libopencsd_c_api.so.1 (0x00007f839e8b2000) libopencsd.so.1 => /usr/local/lib/libopencsd.so.1 (0x00007f839da3c000) ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$ ⬢[acme@toolbox perf]$ ldd /tmp/build/perf/feature/test-libopencsd.bin linux-vdso.so.1 (0x00007ffd669b3000) libopencsd_c_api.so.1 => /usr/local/lib/libopencsd_c_api.so.1 (0x00007fe608b8c000) libopencsd.so.1 => /usr/local/lib/libopencsd.so.1 (0x00007fe608af5000) libc.so.6 => /lib64/libc.so.6 (0x00007fe60891e000) libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007fe6086ff000) libm.so.6 => /lib64/libm.so.6 (0x00007fe6085bb000) libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fe6085a0000) /lib64/ld-linux-x86-64.so.2 (0x00007fe608ba2000) ⬢[acme@toolbox perf]$ ls -la /usr/local/lib/libopencsd* -rw-r--r--. 1 root root 1641364 Sep 1 13:41 /usr/local/lib/libopencsd.a -rw-r--r--. 1 root root 168022 Sep 1 13:41 /usr/local/lib/libopencsd_c_api.a lrwxrwxrwx. 1 root root 21 Sep 1 13:41 /usr/local/lib/libopencsd_c_api.so -> libopencsd_c_api.so.1 lrwxrwxrwx. 1 root root 25 Sep 1 13:41 /usr/local/lib/libopencsd_c_api.so.1 -> libopencsd_c_api.so.1.1.1 -rw-r--r--. 1 root root 104968 Sep 1 13:41 /usr/local/lib/libopencsd_c_api.so.1.1.1 lrwxrwxrwx. 1 root root 15 Sep 1 13:41 /usr/local/lib/libopencsd.so -> libopencsd.so.1 lrwxrwxrwx. 1 root root 19 Sep 1 13:41 /usr/local/lib/libopencsd.so.1 -> libopencsd.so.1.1.1 -rw-r--r--. 1 root root 762432 Sep 1 13:41 /usr/local/lib/libopencsd.so.1.1.1 ⬢[acme@toolbox perf]$
This doesn't explain that 'make -C tools/perf build-test' error, perhaps it is reusing the feature dump (feature detection), done without CORESIGHT=1, when building with CORESIGHT=1 :-\
Anyway, please consider making the build fail when CORESIGHT=1 is passed explicitely and that tools/build/feature-libopencsd.c feature test fails instead of silently building the tool _without_ the explicitely asked for feature.
Thanks,
- Arnaldo
Hi Arnaldo,
On Wed, Sep 01, 2021 at 01:49:51PM -0300, Arnaldo Carvalho de Melo wrote:
[...]
This doesn't explain that 'make -C tools/perf build-test' error, perhaps it is reusing the feature dump (feature detection), done without CORESIGHT=1, when building with CORESIGHT=1 :-\
Anyway, please consider making the build fail when CORESIGHT=1 is passed explicitely and that tools/build/feature-libopencsd.c feature test fails instead of silently building the tool _without_ the explicitely asked for feature.
Sorry for inconvenience.
I have sent a patch to allow the build to report error when detect feature failure for libopencsd [1].
It's obviously that there have another issue with 'make -C tools/perf build-test' which should not build with option 'CORESIGHT=1' by default. Please let me know if you want me to follow this issue or not.
Thanks, Leo
[1] https://lists.linaro.org/pipermail/coresight/2021-September/006967.html p.s. I didn't use the patch linkage from lore.kernel.org, seems the server is down when I try to fetch link from it.
Hi Arnaldo,
On Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo wrote:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
Have you already merged this?
If so than let it be. Otherwise please hold off as I'd like to take a look, something I intend on doing next week.
Thanks, Mathieu
- Arnaldo
tools/perf/util/cs-etm.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 788ad5a099f6..5b276bdb96a6 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2973,6 +2973,11 @@ int cs_etm__process_auxtrace_info(union perf_event *event, /* ETE shares first part of metadata with ETMv4 */ trcidr_idx = CS_ETMV4_TRCTRACEIDR;
} else {ui__error("CS ETM Trace: Unrecognised magic number %#"PRIx64". File could be from a newer version of perf.\n",ptr[i]);err = -EINVAL;goto err_free_metadata;if (!metadata[j]) { -- 2.28.0
--
- Arnaldo
Em Thu, Sep 02, 2021 at 11:48:51AM -0600, Mathieu Poirier escreveu:
Hi Arnaldo,
On Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo wrote:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
Have you already merged this?
If so than let it be. Otherwise please hold off as I'd like to take a look, something I intend on doing next week.
Ok, I can remove them from my local branch, but this may make this miss the v5.15 merge window, please advise.
- Arnaldo
On Thu, 2 Sept 2021 at 12:19, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Thu, Sep 02, 2021 at 11:48:51AM -0600, Mathieu Poirier escreveu:
Hi Arnaldo,
On Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo wrote:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
Have you already merged this?
If so than let it be. Otherwise please hold off as I'd like to take a look, something I intend on doing next week.
Ok, I can remove them from my local branch, but this may make this miss the v5.15 merge window, please advise.
Nah, leave it in your branch and proceed for this merge window.
- Arnaldo
Reviewed-by: Mike Leach mike.leach@linaro.org
On Thu, 2 Sept 2021 at 20:22, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Thu, 2 Sept 2021 at 12:19, Arnaldo Carvalho de Melo acme@kernel.org wrote:
Em Thu, Sep 02, 2021 at 11:48:51AM -0600, Mathieu Poirier escreveu:
Hi Arnaldo,
On Wed, Sep 01, 2021 at 12:54:34PM -0300, Arnaldo Carvalho de Melo wrote:
Em Tue, Aug 24, 2021 at 04:36:15PM +0800, Leo Yan escreveu:
On Fri, Aug 06, 2021 at 02:41:09PM +0100, James Clark wrote:
Currently perf reports "Cannot allocate memory" which isn't very helpful for a potentially user facing issue. If we add a new magic number in the future, perf will be able to report unrecognised magic numbers.
Signed-off-by: James Clark james.clark@arm.com
Reviewed-by: Leo Yan leo.yan@linaro.org
Applies cleanly to my tree, test building it now, holler if there is something that prevents it from being merged.
Have you already merged this?
If so than let it be. Otherwise please hold off as I'd like to take a look, something I intend on doing next week.
Ok, I can remove them from my local branch, but this may make this miss the v5.15 merge window, please advise.
Nah, leave it in your branch and proceed for this merge window.
- Arnaldo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Hi Mathieu, Suzuki, Mike,
On Fri, Aug 06, 2021 at 02:41:00PM +0100, James Clark wrote:
Changes since v1:
- Re-implement with a new magic number instead of piggybacking on ETMv4
- Improve comments and function name around cs_etm_decoder__get_etmv4_arch_ver()
- Add a warning for unrecognised magic numbers
- Split typo fix into new commit
- Add Leo's reviewed-by tags
- Create a new struct for ETE config (cs_ete_trace_params) instead of re-using ETMv4 config
Applies to perf/core f3c33cbd922 Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-ete-v2
I have finished the review for this patch series and it's good form for me. Since this patch series is to enable new feature for ETE, it's good if any of you could review as well before merging.
Thanks, Leo
On 24/08/2021 09:47, Leo Yan wrote:
Hi Mathieu, Suzuki, Mike,
On Fri, Aug 06, 2021 at 02:41:00PM +0100, James Clark wrote:
Changes since v1:
- Re-implement with a new magic number instead of piggybacking on ETMv4
- Improve comments and function name around cs_etm_decoder__get_etmv4_arch_ver()
- Add a warning for unrecognised magic numbers
- Split typo fix into new commit
- Add Leo's reviewed-by tags
- Create a new struct for ETE config (cs_ete_trace_params) instead of re-using ETMv4 config
Applies to perf/core f3c33cbd922 Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-ete-v2
I have finished the review for this patch series and it's good form for me. Since this patch series is to enable new feature for ETE, it's good if any of you could review as well before merging.
Thanks for the reviews Leo!
Thanks, Leo
-
Arnaldo Carvalho de Melo -
James Clark -
Leo Yan -
Mathieu Poirier -
Mike Leach -
Suzuki K Poulose