* [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* 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: 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
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: 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
@ 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: 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-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: 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-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: 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
0 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
@ 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: 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
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: 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
@ 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, 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
* 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, 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
* [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