All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-15  8:28 ` Leo Yan
  0 siblings, 0 replies; 13+ messages in thread
From: Leo Yan @ 2019-08-15  8:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Mathieu Poirier, Suzuki K Poulose,
	linux-arm-kernel, linux-kernel
  Cc: Leo Yan, Mike Leach, Robert Walker, coresight

The synthetic branch and instruction samples are missed to set
instruction related info, thus perf tool fails to display samples with
flags '-F,+insn,+insnlen'.

CoreSight trace decoder has provided sufficient information to decide
the instruction size based on the isa type: A64/A32 instruction are
32-bit size, but one exception is the T32 instruction size, which might
be 32-bit or 16-bit.

This patch handles for these cases and it reads the instruction values
from DSO file; thus can support flags '-F,+insn,+insnlen'.

Before:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0

  [...]

After:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 4 insn: 2f 02 00 94
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54

  [...]

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Robert Walker <robert.walker@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ed6f7fd5b90b..b3a5daaf1a8f 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
 	return !!etmq->etm->timeless_decoding;
 }
 
+static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
+			      u64 trace_chan_id,
+			      const struct cs_etm_packet *packet,
+			      struct perf_sample *sample)
+{
+	/*
+	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
+	 * packet, so directly bail out with 'insn_len' = 0.
+	 */
+	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
+		sample->insn_len = 0;
+		return;
+	}
+
+	/*
+	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
+	 * cs_etm__t32_instr_size().
+	 */
+	if (packet->isa == CS_ETM_ISA_T32)
+		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
+							  sample->ip);
+	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
+	else
+		sample->insn_len = 4;
+
+	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
+			   sample->insn_len, (void *)sample->insn);
+}
+
 static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 					    struct cs_etm_traceid_queue *tidq,
 					    u64 addr, u64 period)
@@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 	sample.period = period;
 	sample.cpu = tidq->packet->cpu;
 	sample.flags = tidq->prev_packet->flags;
-	sample.insn_len = 1;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
+
 	if (etm->synth_opts.last_branch) {
 		cs_etm__copy_last_branch_rb(etmq, tidq);
 		sample.branch_stack = tidq->last_branch;
@@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
 	sample.flags = tidq->prev_packet->flags;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
+			  &sample);
+
 	/*
 	 * perf report cannot handle events without a branch stack
 	 */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-15  8:28 ` Leo Yan
  0 siblings, 0 replies; 13+ messages in thread
From: Leo Yan @ 2019-08-15  8:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Mathieu Poirier, Suzuki K Poulose,
	linux-arm-kernel, linux-kernel
  Cc: Robert Walker, Mike Leach, coresight, Leo Yan

The synthetic branch and instruction samples are missed to set
instruction related info, thus perf tool fails to display samples with
flags '-F,+insn,+insnlen'.

CoreSight trace decoder has provided sufficient information to decide
the instruction size based on the isa type: A64/A32 instruction are
32-bit size, but one exception is the T32 instruction size, which might
be 32-bit or 16-bit.

This patch handles for these cases and it reads the instruction values
from DSO file; thus can support flags '-F,+insn,+insnlen'.

Before:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0

  [...]

After:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 4 insn: 2f 02 00 94
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54

  [...]

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Robert Walker <robert.walker@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ed6f7fd5b90b..b3a5daaf1a8f 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
 	return !!etmq->etm->timeless_decoding;
 }
 
+static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
+			      u64 trace_chan_id,
+			      const struct cs_etm_packet *packet,
+			      struct perf_sample *sample)
+{
+	/*
+	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
+	 * packet, so directly bail out with 'insn_len' = 0.
+	 */
+	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
+		sample->insn_len = 0;
+		return;
+	}
+
+	/*
+	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
+	 * cs_etm__t32_instr_size().
+	 */
+	if (packet->isa == CS_ETM_ISA_T32)
+		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
+							  sample->ip);
+	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
+	else
+		sample->insn_len = 4;
+
+	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
+			   sample->insn_len, (void *)sample->insn);
+}
+
 static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 					    struct cs_etm_traceid_queue *tidq,
 					    u64 addr, u64 period)
@@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 	sample.period = period;
 	sample.cpu = tidq->packet->cpu;
 	sample.flags = tidq->prev_packet->flags;
-	sample.insn_len = 1;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
+
 	if (etm->synth_opts.last_branch) {
 		cs_etm__copy_last_branch_rb(etmq, tidq);
 		sample.branch_stack = tidq->last_branch;
@@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
 	sample.flags = tidq->prev_packet->flags;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
+			  &sample);
+
 	/*
 	 * perf report cannot handle events without a branch stack
 	 */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-15  8:28 ` Leo Yan
@ 2019-08-19 14:23   ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-19 14:23 UTC (permalink / raw)
  To: Mathieu Poirier, Leo Yan
  Cc: Alexander Shishkin, Jiri Olsa, Namhyung Kim, Suzuki K Poulose,
	linux-arm-kernel, linux-kernel, Mike Leach, Robert Walker,
	coresight

Em Thu, Aug 15, 2019 at 04:28:54PM +0800, Leo Yan escreveu:
> The synthetic branch and instruction samples are missed to set
> instruction related info, thus perf tool fails to display samples with
> flags '-F,+insn,+insnlen'.
> 
> CoreSight trace decoder has provided sufficient information to decide
> the instruction size based on the isa type: A64/A32 instruction are
> 32-bit size, but one exception is the T32 instruction size, which might
> be 32-bit or 16-bit.
> 
> This patch handles for these cases and it reads the instruction values
> from DSO file; thus can support flags '-F,+insn,+insnlen'.

Mathieu, can I have your Acked-by/Reviewed-by?

- Arnaldo
 
> Before:
> 
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
> 
>   [...]
> 
> After:
> 
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> 
>   [...]
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index ed6f7fd5b90b..b3a5daaf1a8f 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
>  	return !!etmq->etm->timeless_decoding;
>  }
>  
> +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> +			      u64 trace_chan_id,
> +			      const struct cs_etm_packet *packet,
> +			      struct perf_sample *sample)
> +{
> +	/*
> +	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> +	 * packet, so directly bail out with 'insn_len' = 0.
> +	 */
> +	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> +		sample->insn_len = 0;
> +		return;
> +	}
> +
> +	/*
> +	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
> +	 * cs_etm__t32_instr_size().
> +	 */
> +	if (packet->isa == CS_ETM_ISA_T32)
> +		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> +							  sample->ip);
> +	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
> +	else
> +		sample->insn_len = 4;
> +
> +	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> +			   sample->insn_len, (void *)sample->insn);
> +}
> +
>  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  					    struct cs_etm_traceid_queue *tidq,
>  					    u64 addr, u64 period)
> @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  	sample.period = period;
>  	sample.cpu = tidq->packet->cpu;
>  	sample.flags = tidq->prev_packet->flags;
> -	sample.insn_len = 1;
>  	sample.cpumode = event->sample.header.misc;
>  
> +	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> +
>  	if (etm->synth_opts.last_branch) {
>  		cs_etm__copy_last_branch_rb(etmq, tidq);
>  		sample.branch_stack = tidq->last_branch;
> @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
>  	sample.flags = tidq->prev_packet->flags;
>  	sample.cpumode = event->sample.header.misc;
>  
> +	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> +			  &sample);
> +
>  	/*
>  	 * perf report cannot handle events without a branch stack
>  	 */
> -- 
> 2.17.1

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-19 14:23   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-19 14:23 UTC (permalink / raw)
  To: Mathieu Poirier, Leo Yan
  Cc: Suzuki K Poulose, Alexander Shishkin, coresight, linux-kernel,
	Namhyung Kim, Robert Walker, Jiri Olsa, linux-arm-kernel,
	Mike Leach

Em Thu, Aug 15, 2019 at 04:28:54PM +0800, Leo Yan escreveu:
> The synthetic branch and instruction samples are missed to set
> instruction related info, thus perf tool fails to display samples with
> flags '-F,+insn,+insnlen'.
> 
> CoreSight trace decoder has provided sufficient information to decide
> the instruction size based on the isa type: A64/A32 instruction are
> 32-bit size, but one exception is the T32 instruction size, which might
> be 32-bit or 16-bit.
> 
> This patch handles for these cases and it reads the instruction values
> from DSO file; thus can support flags '-F,+insn,+insnlen'.

Mathieu, can I have your Acked-by/Reviewed-by?

- Arnaldo
 
> Before:
> 
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
> 
>   [...]
> 
> After:
> 
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> 
>   [...]
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index ed6f7fd5b90b..b3a5daaf1a8f 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
>  	return !!etmq->etm->timeless_decoding;
>  }
>  
> +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> +			      u64 trace_chan_id,
> +			      const struct cs_etm_packet *packet,
> +			      struct perf_sample *sample)
> +{
> +	/*
> +	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> +	 * packet, so directly bail out with 'insn_len' = 0.
> +	 */
> +	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> +		sample->insn_len = 0;
> +		return;
> +	}
> +
> +	/*
> +	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
> +	 * cs_etm__t32_instr_size().
> +	 */
> +	if (packet->isa == CS_ETM_ISA_T32)
> +		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> +							  sample->ip);
> +	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
> +	else
> +		sample->insn_len = 4;
> +
> +	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> +			   sample->insn_len, (void *)sample->insn);
> +}
> +
>  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  					    struct cs_etm_traceid_queue *tidq,
>  					    u64 addr, u64 period)
> @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  	sample.period = period;
>  	sample.cpu = tidq->packet->cpu;
>  	sample.flags = tidq->prev_packet->flags;
> -	sample.insn_len = 1;
>  	sample.cpumode = event->sample.header.misc;
>  
> +	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> +
>  	if (etm->synth_opts.last_branch) {
>  		cs_etm__copy_last_branch_rb(etmq, tidq);
>  		sample.branch_stack = tidq->last_branch;
> @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
>  	sample.flags = tidq->prev_packet->flags;
>  	sample.cpumode = event->sample.header.misc;
>  
> +	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> +			  &sample);
> +
>  	/*
>  	 * perf report cannot handle events without a branch stack
>  	 */
> -- 
> 2.17.1

-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-19 14:23   ` Arnaldo Carvalho de Melo
@ 2019-08-19 14:36     ` Mathieu Poirier
  -1 siblings, 0 replies; 13+ messages in thread
From: Mathieu Poirier @ 2019-08-19 14:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Leo Yan, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Suzuki K Poulose, linux-arm-kernel, Linux Kernel Mailing List,
	Mike Leach, Robert Walker, Coresight ML

On Mon, 19 Aug 2019 at 08:23, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Aug 15, 2019 at 04:28:54PM +0800, Leo Yan escreveu:
> > The synthetic branch and instruction samples are missed to set
> > instruction related info, thus perf tool fails to display samples with
> > flags '-F,+insn,+insnlen'.
> >
> > CoreSight trace decoder has provided sufficient information to decide
> > the instruction size based on the isa type: A64/A32 instruction are
> > 32-bit size, but one exception is the T32 instruction size, which might
> > be 32-bit or 16-bit.
> >
> > This patch handles for these cases and it reads the instruction values
> > from DSO file; thus can support flags '-F,+insn,+insnlen'.
>
> Mathieu, can I have your Acked-by/Reviewed-by?

Yes, as soon as I have the opportunity to test it.

>
> - Arnaldo
>
> > Before:
> >
> >   # perf script -F,insn,insnlen,ip,sym
> >                 0 [unknown] ilen: 0
> >      ffff97174044 _start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >
> >   [...]
> >
> > After:
> >
> >   # perf script -F,insn,insnlen,ip,sym
> >                 0 [unknown] ilen: 0
> >      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >
> >   [...]
> >
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> > Cc: Mike Leach <mike.leach@linaro.org>
> > Cc: Robert Walker <robert.walker@arm.com>
> > Cc: coresight@lists.linaro.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index ed6f7fd5b90b..b3a5daaf1a8f 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
> >       return !!etmq->etm->timeless_decoding;
> >  }
> >
> > +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> > +                           u64 trace_chan_id,
> > +                           const struct cs_etm_packet *packet,
> > +                           struct perf_sample *sample)
> > +{
> > +     /*
> > +      * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> > +      * packet, so directly bail out with 'insn_len' = 0.
> > +      */
> > +     if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> > +             sample->insn_len = 0;
> > +             return;
> > +     }
> > +
> > +     /*
> > +      * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > +      * cs_etm__t32_instr_size().
> > +      */
> > +     if (packet->isa == CS_ETM_ISA_T32)
> > +             sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> > +                                                       sample->ip);
> > +     /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > +     else
> > +             sample->insn_len = 4;
> > +
> > +     cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> > +                        sample->insn_len, (void *)sample->insn);
> > +}
> > +
> >  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> >                                           struct cs_etm_traceid_queue *tidq,
> >                                           u64 addr, u64 period)
> > @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> >       sample.period = period;
> >       sample.cpu = tidq->packet->cpu;
> >       sample.flags = tidq->prev_packet->flags;
> > -     sample.insn_len = 1;
> >       sample.cpumode = event->sample.header.misc;
> >
> > +     cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> > +
> >       if (etm->synth_opts.last_branch) {
> >               cs_etm__copy_last_branch_rb(etmq, tidq);
> >               sample.branch_stack = tidq->last_branch;
> > @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
> >       sample.flags = tidq->prev_packet->flags;
> >       sample.cpumode = event->sample.header.misc;
> >
> > +     cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> > +                       &sample);
> > +
> >       /*
> >        * perf report cannot handle events without a branch stack
> >        */
> > --
> > 2.17.1
>
> --
>
> - Arnaldo

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-19 14:36     ` Mathieu Poirier
  0 siblings, 0 replies; 13+ messages in thread
From: Mathieu Poirier @ 2019-08-19 14:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Suzuki K Poulose, Alexander Shishkin, Coresight ML,
	Linux Kernel Mailing List, Leo Yan, Namhyung Kim, Robert Walker,
	Jiri Olsa, linux-arm-kernel, Mike Leach

On Mon, 19 Aug 2019 at 08:23, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Aug 15, 2019 at 04:28:54PM +0800, Leo Yan escreveu:
> > The synthetic branch and instruction samples are missed to set
> > instruction related info, thus perf tool fails to display samples with
> > flags '-F,+insn,+insnlen'.
> >
> > CoreSight trace decoder has provided sufficient information to decide
> > the instruction size based on the isa type: A64/A32 instruction are
> > 32-bit size, but one exception is the T32 instruction size, which might
> > be 32-bit or 16-bit.
> >
> > This patch handles for these cases and it reads the instruction values
> > from DSO file; thus can support flags '-F,+insn,+insnlen'.
>
> Mathieu, can I have your Acked-by/Reviewed-by?

Yes, as soon as I have the opportunity to test it.

>
> - Arnaldo
>
> > Before:
> >
> >   # perf script -F,insn,insnlen,ip,sym
> >                 0 [unknown] ilen: 0
> >      ffff97174044 _start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >      ffff97174938 _dl_start ilen: 0
> >
> >   [...]
> >
> > After:
> >
> >   # perf script -F,insn,insnlen,ip,sym
> >                 0 [unknown] ilen: 0
> >      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
> >
> >   [...]
> >
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> > Cc: Mike Leach <mike.leach@linaro.org>
> > Cc: Robert Walker <robert.walker@arm.com>
> > Cc: coresight@lists.linaro.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index ed6f7fd5b90b..b3a5daaf1a8f 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
> >       return !!etmq->etm->timeless_decoding;
> >  }
> >
> > +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> > +                           u64 trace_chan_id,
> > +                           const struct cs_etm_packet *packet,
> > +                           struct perf_sample *sample)
> > +{
> > +     /*
> > +      * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> > +      * packet, so directly bail out with 'insn_len' = 0.
> > +      */
> > +     if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> > +             sample->insn_len = 0;
> > +             return;
> > +     }
> > +
> > +     /*
> > +      * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > +      * cs_etm__t32_instr_size().
> > +      */
> > +     if (packet->isa == CS_ETM_ISA_T32)
> > +             sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> > +                                                       sample->ip);
> > +     /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > +     else
> > +             sample->insn_len = 4;
> > +
> > +     cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> > +                        sample->insn_len, (void *)sample->insn);
> > +}
> > +
> >  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> >                                           struct cs_etm_traceid_queue *tidq,
> >                                           u64 addr, u64 period)
> > @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> >       sample.period = period;
> >       sample.cpu = tidq->packet->cpu;
> >       sample.flags = tidq->prev_packet->flags;
> > -     sample.insn_len = 1;
> >       sample.cpumode = event->sample.header.misc;
> >
> > +     cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> > +
> >       if (etm->synth_opts.last_branch) {
> >               cs_etm__copy_last_branch_rb(etmq, tidq);
> >               sample.branch_stack = tidq->last_branch;
> > @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
> >       sample.flags = tidq->prev_packet->flags;
> >       sample.cpumode = event->sample.header.misc;
> >
> > +     cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> > +                       &sample);
> > +
> >       /*
> >        * perf report cannot handle events without a branch stack
> >        */
> > --
> > 2.17.1
>
> --
>
> - Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-15  8:28 ` Leo Yan
@ 2019-08-19 18:08   ` Mathieu Poirier
  -1 siblings, 0 replies; 13+ messages in thread
From: Mathieu Poirier @ 2019-08-19 18:08 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel,
	Linux Kernel Mailing List, Mike Leach, Robert Walker,
	Coresight ML

On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
>
> The synthetic branch and instruction samples are missed to set
> instruction related info, thus perf tool fails to display samples with
> flags '-F,+insn,+insnlen'.
>
> CoreSight trace decoder has provided sufficient information to decide
> the instruction size based on the isa type: A64/A32 instruction are
> 32-bit size, but one exception is the T32 instruction size, which might
> be 32-bit or 16-bit.
>
> This patch handles for these cases and it reads the instruction values
> from DSO file; thus can support flags '-F,+insn,+insnlen'.
>
> Before:
>
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>
>   [...]
>
> After:
>
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>
>   [...]
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index ed6f7fd5b90b..b3a5daaf1a8f 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
>         return !!etmq->etm->timeless_decoding;
>  }
>
> +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> +                             u64 trace_chan_id,
> +                             const struct cs_etm_packet *packet,
> +                             struct perf_sample *sample)
> +{
> +       /*
> +        * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> +        * packet, so directly bail out with 'insn_len' = 0.
> +        */
> +       if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> +               sample->insn_len = 0;
> +               return;
> +       }
> +
> +       /*
> +        * T32 instruction size might be 32-bit or 16-bit, decide by calling
> +        * cs_etm__t32_instr_size().
> +        */
> +       if (packet->isa == CS_ETM_ISA_T32)
> +               sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> +                                                         sample->ip);
> +       /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> +       else
> +               sample->insn_len = 4;
> +
> +       cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> +                          sample->insn_len, (void *)sample->insn);
> +}
> +
>  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>                                             struct cs_etm_traceid_queue *tidq,
>                                             u64 addr, u64 period)
> @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>         sample.period = period;
>         sample.cpu = tidq->packet->cpu;
>         sample.flags = tidq->prev_packet->flags;
> -       sample.insn_len = 1;
>         sample.cpumode = event->sample.header.misc;
>
> +       cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> +
>         if (etm->synth_opts.last_branch) {
>                 cs_etm__copy_last_branch_rb(etmq, tidq);
>                 sample.branch_stack = tidq->last_branch;
> @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
>         sample.flags = tidq->prev_packet->flags;
>         sample.cpumode = event->sample.header.misc;
>
> +       cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> +                         &sample);
> +

The code seems to be correct.  I have also tested this patch.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>         /*
>          * perf report cannot handle events without a branch stack
>          */
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-19 18:08   ` Mathieu Poirier
  0 siblings, 0 replies; 13+ messages in thread
From: Mathieu Poirier @ 2019-08-19 18:08 UTC (permalink / raw)
  To: Leo Yan
  Cc: Suzuki K Poulose, Alexander Shishkin, Coresight ML,
	Linux Kernel Mailing List, Arnaldo Carvalho de Melo,
	Namhyung Kim, Robert Walker, Jiri Olsa, linux-arm-kernel,
	Mike Leach

On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
>
> The synthetic branch and instruction samples are missed to set
> instruction related info, thus perf tool fails to display samples with
> flags '-F,+insn,+insnlen'.
>
> CoreSight trace decoder has provided sufficient information to decide
> the instruction size based on the isa type: A64/A32 instruction are
> 32-bit size, but one exception is the T32 instruction size, which might
> be 32-bit or 16-bit.
>
> This patch handles for these cases and it reads the instruction values
> from DSO file; thus can support flags '-F,+insn,+insnlen'.
>
> Before:
>
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>      ffff97174938 _dl_start ilen: 0
>
>   [...]
>
> After:
>
>   # perf script -F,insn,insnlen,ip,sym
>                 0 [unknown] ilen: 0
>      ffff97174044 _start ilen: 4 insn: 2f 02 00 94
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>      ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
>
>   [...]
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index ed6f7fd5b90b..b3a5daaf1a8f 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
>         return !!etmq->etm->timeless_decoding;
>  }
>
> +static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> +                             u64 trace_chan_id,
> +                             const struct cs_etm_packet *packet,
> +                             struct perf_sample *sample)
> +{
> +       /*
> +        * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
> +        * packet, so directly bail out with 'insn_len' = 0.
> +        */
> +       if (packet->sample_type == CS_ETM_DISCONTINUITY) {
> +               sample->insn_len = 0;
> +               return;
> +       }
> +
> +       /*
> +        * T32 instruction size might be 32-bit or 16-bit, decide by calling
> +        * cs_etm__t32_instr_size().
> +        */
> +       if (packet->isa == CS_ETM_ISA_T32)
> +               sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> +                                                         sample->ip);
> +       /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> +       else
> +               sample->insn_len = 4;
> +
> +       cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> +                          sample->insn_len, (void *)sample->insn);
> +}
> +
>  static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>                                             struct cs_etm_traceid_queue *tidq,
>                                             u64 addr, u64 period)
> @@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>         sample.period = period;
>         sample.cpu = tidq->packet->cpu;
>         sample.flags = tidq->prev_packet->flags;
> -       sample.insn_len = 1;
>         sample.cpumode = event->sample.header.misc;
>
> +       cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> +
>         if (etm->synth_opts.last_branch) {
>                 cs_etm__copy_last_branch_rb(etmq, tidq);
>                 sample.branch_stack = tidq->last_branch;
> @@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
>         sample.flags = tidq->prev_packet->flags;
>         sample.cpumode = event->sample.header.misc;
>
> +       cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
> +                         &sample);
> +

The code seems to be correct.  I have also tested this patch.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>         /*
>          * perf report cannot handle events without a branch stack
>          */
> --
> 2.17.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-19 18:08   ` Mathieu Poirier
@ 2019-08-19 18:50     ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-19 18:50 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Leo Yan, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Suzuki K Poulose, linux-arm-kernel, Linux Kernel Mailing List,
	Mike Leach, Robert Walker, Coresight ML

Em Mon, Aug 19, 2019 at 12:08:26PM -0600, Mathieu Poirier escreveu:
> On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
> >
> > The synthetic branch and instruction samples are missed to set
> > instruction related info, thus perf tool fails to display samples with
> > flags '-F,+insn,+insnlen'.
> >
> > CoreSight trace decoder has provided sufficient information to decide
> > the instruction size based on the isa type: A64/A32 instruction are
> > 32-bit size, but one exception is the T32 instruction size, which might
> > be 32-bit or 16-bit.
> >
> > This patch handles for these cases and it reads the instruction values
> > from DSO file; thus can support flags '-F,+insn,+insnlen'.
 
> The code seems to be correct.  I have also tested this patch.
 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-19 18:50     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-19 18:50 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Alexander Shishkin, Coresight ML,
	Linux Kernel Mailing List, Leo Yan, Namhyung Kim, Robert Walker,
	Jiri Olsa, linux-arm-kernel, Mike Leach

Em Mon, Aug 19, 2019 at 12:08:26PM -0600, Mathieu Poirier escreveu:
> On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
> >
> > The synthetic branch and instruction samples are missed to set
> > instruction related info, thus perf tool fails to display samples with
> > flags '-F,+insn,+insnlen'.
> >
> > CoreSight trace decoder has provided sufficient information to decide
> > the instruction size based on the isa type: A64/A32 instruction are
> > 32-bit size, but one exception is the T32 instruction size, which might
> > be 32-bit or 16-bit.
> >
> > This patch handles for these cases and it reads the instruction values
> > from DSO file; thus can support flags '-F,+insn,+insnlen'.
 
> The code seems to be correct.  I have also tested this patch.
 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks, applied.

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-19 18:50     ` Arnaldo Carvalho de Melo
@ 2019-08-20  1:12       ` Leo Yan
  -1 siblings, 0 replies; 13+ messages in thread
From: Leo Yan @ 2019-08-20  1:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Mathieu Poirier, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Suzuki K Poulose, linux-arm-kernel, Linux Kernel Mailing List,
	Mike Leach, Robert Walker, Coresight ML

On Mon, Aug 19, 2019 at 03:50:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 19, 2019 at 12:08:26PM -0600, Mathieu Poirier escreveu:
> > On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
> > >
> > > The synthetic branch and instruction samples are missed to set
> > > instruction related info, thus perf tool fails to display samples with
> > > flags '-F,+insn,+insnlen'.
> > >
> > > CoreSight trace decoder has provided sufficient information to decide
> > > the instruction size based on the isa type: A64/A32 instruction are
> > > 32-bit size, but one exception is the T32 instruction size, which might
> > > be 32-bit or 16-bit.
> > >
> > > This patch handles for these cases and it reads the instruction values
> > > from DSO file; thus can support flags '-F,+insn,+insnlen'.
>  
> > The code seems to be correct.  I have also tested this patch.
>  
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Thanks, applied.

Thanks a lot, Mathieu & Arnaldo.

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen'
@ 2019-08-20  1:12       ` Leo Yan
  0 siblings, 0 replies; 13+ messages in thread
From: Leo Yan @ 2019-08-20  1:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin,
	Coresight ML, Linux Kernel Mailing List, Namhyung Kim,
	Robert Walker, Jiri Olsa, linux-arm-kernel, Mike Leach

On Mon, Aug 19, 2019 at 03:50:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 19, 2019 at 12:08:26PM -0600, Mathieu Poirier escreveu:
> > On Thu, 15 Aug 2019 at 02:30, Leo Yan <leo.yan@linaro.org> wrote:
> > >
> > > The synthetic branch and instruction samples are missed to set
> > > instruction related info, thus perf tool fails to display samples with
> > > flags '-F,+insn,+insnlen'.
> > >
> > > CoreSight trace decoder has provided sufficient information to decide
> > > the instruction size based on the isa type: A64/A32 instruction are
> > > 32-bit size, but one exception is the T32 instruction size, which might
> > > be 32-bit or 16-bit.
> > >
> > > This patch handles for these cases and it reads the instruction values
> > > from DSO file; thus can support flags '-F,+insn,+insnlen'.
>  
> > The code seems to be correct.  I have also tested this patch.
>  
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Thanks, applied.

Thanks a lot, Mathieu & Arnaldo.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [tip: perf/core] perf cs-etm: Support sample flags 'insn' and 'insnlen'
  2019-08-15  8:28 ` Leo Yan
                   ` (2 preceding siblings ...)
  (?)
@ 2019-08-23  2:28 ` tip-bot2 for Leo Yan
  -1 siblings, 0 replies; 13+ messages in thread
From: tip-bot2 for Leo Yan @ 2019-08-23  2:28 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, Arnaldo Carvalho de Melo, linux-arm-kernel,
	coresight, Suzuki Poulouse, Robert Walker, Namhyung Kim,
	Mike Leach, Jiri Olsa, Alexander Shishkin, Mathieu Poirier,
	Leo Yan

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     a4973d8f7bea98a0795ba853e7bd2cef11363824
Gitweb:        https://git.kernel.org/tip/a4973d8f7bea98a0795ba853e7bd2cef11363824
Author:        Leo Yan <leo.yan@linaro.org>
AuthorDate:    Thu, 15 Aug 2019 16:28:54 +08:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 20 Aug 2019 12:20:52 -03:00

perf cs-etm: Support sample flags 'insn' and 'insnlen'

The synthetic branch and instruction samples are missed to set
instruction related info, thus the perf tool fails to display samples
with flags '-F,+insn,+insnlen'.

The CoreSight trace decoder provides sufficient information to decide
the instruction size based on the ISA type: A64/A32 instructions are
32-bit size, but one exception is the T32 instruction size, which might
be 32-bit or 16-bit.

This patch handles these cases and it reads the instruction values from
DSO file; thus can support the flags '-F,+insn,+insnlen'.

Before:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0

  [...]

After:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 4 insn: 2f 02 00 94
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54

  [...]

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Robert Walker <robert.walker@arm.com>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20190815082854.18191-1-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ed6f7fd..b3a5daa 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
 	return !!etmq->etm->timeless_decoding;
 }
 
+static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
+			      u64 trace_chan_id,
+			      const struct cs_etm_packet *packet,
+			      struct perf_sample *sample)
+{
+	/*
+	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
+	 * packet, so directly bail out with 'insn_len' = 0.
+	 */
+	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
+		sample->insn_len = 0;
+		return;
+	}
+
+	/*
+	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
+	 * cs_etm__t32_instr_size().
+	 */
+	if (packet->isa == CS_ETM_ISA_T32)
+		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
+							  sample->ip);
+	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
+	else
+		sample->insn_len = 4;
+
+	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
+			   sample->insn_len, (void *)sample->insn);
+}
+
 static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 					    struct cs_etm_traceid_queue *tidq,
 					    u64 addr, u64 period)
@@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 	sample.period = period;
 	sample.cpu = tidq->packet->cpu;
 	sample.flags = tidq->prev_packet->flags;
-	sample.insn_len = 1;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
+
 	if (etm->synth_opts.last_branch) {
 		cs_etm__copy_last_branch_rb(etmq, tidq);
 		sample.branch_stack = tidq->last_branch;
@@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
 	sample.flags = tidq->prev_packet->flags;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
+			  &sample);
+
 	/*
 	 * perf report cannot handle events without a branch stack
 	 */

^ permalink raw reply related	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-23  2:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15  8:28 [PATCH 1/2] perf cs-etm: Support sample flags 'insn' and 'insnlen' Leo Yan
2019-08-15  8:28 ` Leo Yan
2019-08-19 14:23 ` Arnaldo Carvalho de Melo
2019-08-19 14:23   ` Arnaldo Carvalho de Melo
2019-08-19 14:36   ` Mathieu Poirier
2019-08-19 14:36     ` Mathieu Poirier
2019-08-19 18:08 ` Mathieu Poirier
2019-08-19 18:08   ` Mathieu Poirier
2019-08-19 18:50   ` Arnaldo Carvalho de Melo
2019-08-19 18:50     ` Arnaldo Carvalho de Melo
2019-08-20  1:12     ` Leo Yan
2019-08-20  1:12       ` Leo Yan
2019-08-23  2:28 ` [tip: perf/core] " tip-bot2 for Leo Yan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.