2018-12-11 07:41:36

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 0/8] perf cs-etm: Correct packets handling

perf cs-etm module converts decoder elements to packets and then we have
more context crossing packets to generate synthenize samples, finally
perf tool can faciliate samples for statistics and report the results.

This patch series is to address several issues found related with
packets handling and samples generation when worked firstly on branch
sample flags support for Arm CoreSight trace data, so this patch series
is dependency for sample flags setting, will send another dedicated
patch series for sample flags later.

In this patch series, the first two patches are mainly to fix issues in
cs_etm__flush(): patch 0001 corrects packets swapping in cs_etm__flush()
and this can fix the wrong branch sample caused by the missed packets
swapping; patch 0002 is to fix the wrong samples generation with stale
packets at the end of trace block.

Patch 0003 and 0004 are for minor fixing; patch 0003 removes unused field
'cs_etm_decoder::trace_on', this can simplize the switch-case code for all
discontinuity packet generation by using one code block; patch 0004 is to
refactor enumeration cs_etm_sample_type.

Patch 0005 is to rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY, we use
a more general packet type to present trace discontinuity, so it can be
used by TRACE_ON event, and also can be used by NO_SYNC and EO_TRACE
elements.

Patch 0006 is used to support NO_SYNC packet, otherwise the trace
decoding cannot reflect the tracing discontinuity caused by NO_SYNC
packet.

Patch 0007 is used to support EO_TRACE packet, which also introduces
the tracing discontinuity at the end of trace and we should save last
trace data for it.

Patch 0008 is used to generate branch sample for exception packets.

Credit to Mike Leach and Robert Walker who made me clear for underlying
mechanism for NO_SYNC/EO_TRACE elements, Mike also shared the detailed
explanation for why we can treat NO_SYNC and TRACE_ON elements as the
same, so except following Mike & Rob suggestion for trace discontinuity
consolidation, most commit log of patches 0006/0007 also come from
Mike's explanation.

This patch series is applied directly on the acme's perf/core branch [1]
with latest commit aaab25f03e9e ("perf trace: Allow selecting use the
use of the ordered_events code").

With applying the dependency patch, this patch series has been tested
for branch samples dumping with below command on Juno board:

# perf script -F,-time,+ip,+sym,+dso,+addr,+symoff -k vmlinux

Changes from v2:
* Addressed Mathieu's comments and suggestions for minor refactoring
for removing unused field 'cs_etm_decoder::trace_on' and added one
dedicated patch to refactor enumeration cs_etm_sample_type.
* Added Mathieu's 'reviewed' tags; Very appreciate Mathieu's many
suggestion for crossing several patch series.

Changes from v1:
* Synced the consistent code in patch 0001 for condition checking.
* Introduced new function cs_etm__end_block() for flushing packet
at the end of trace block.
* Added new patch 0003 to rename CS_ETM_TRACE_ON to
CS_ETM_DISCONTINUITY.
* Used the same one packet type CS_ETM_DISCONTINUITY for all
trace discontinuity (include support TRACE_ON/EO_TRACE/NO_SYNC
packets).
* Removed tracking exception number patch, which will be added in
sample flag patch series.


Leo Yan (8):
perf cs-etm: Correct packets swapping in cs_etm__flush()
perf cs-etm: Avoid stale branch samples when flush packet
perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder
perf cs-etm: Refactor enumeration cs_etm_sample_type
perf cs-etm: Rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY
perf cs-etm: Treat NO_SYNC element as trace discontinuity
perf cs-etm: Treat EO_TRACE element as trace discontinuity
perf cs-etm: Generate branch sample for exception packet

tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 42 +++++++++-----
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 10 ++--
tools/perf/util/cs-etm.c | 77 ++++++++++++++++++++++---
3 files changed, 100 insertions(+), 29 deletions(-)

--
2.7.4



2018-12-11 07:41:43

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 1/8] perf cs-etm: Correct packets swapping in cs_etm__flush()

The structure cs_etm_queue uses 'prev_packet' to point to previous
packet, this can be used to combine with new coming packet to generate
samples.

In function cs_etm__flush() it swaps packets only when the flag
'etm->synth_opts.last_branch' is true, this means that it will not
swap packets if without option '--itrace=il' to generate last branch
entries; thus for this case the 'prev_packet' doesn't point to the
correct previous packet and the stale packet still will be used to
generate sequential sample. Thus if dump trace with 'perf script'
command we can see the incorrect flow with the stale packet's address
info.

This patch corrects packets swapping in cs_etm__flush(); except using
the flag 'etm->synth_opts.last_branch' it also checks the another flag
'etm->sample_branches', if any flag is true then it swaps packets so
can save correct content to 'prev_packet'. Finally this can fix the
wrong program flow dumping issue.

The patch has a minor refactoring to use 'etm->synth_opts.last_branch'
instead of 'etmq->etm->synth_opts.last_branch' for condition checking,
this is consistent with that is done in cs_etm__sample().

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 23159c33..789707b 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1042,7 +1042,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
}

swap_packet:
- if (etmq->etm->synth_opts.last_branch) {
+ if (etm->sample_branches || etm->synth_opts.last_branch) {
/*
* Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
* the next incoming packet.
--
2.7.4


2018-12-11 07:41:55

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 2/8] perf cs-etm: Avoid stale branch samples when flush packet

At the end of trace buffer handling, function cs_etm__flush() is invoked
to flush any remaining branch stack entries. As a side effect, it also
generates branch sample, because the 'etmq->packet' doesn't contains any
new coming packet but point to one stale packet after packets swapping,
so it wrongly makes synthesize branch samples with stale packet info.

We could review below detailed flow which causes issue:

Packet1: start_addr=0xffff000008b1fbf0 end_addr=0xffff000008b1fbfc
Packet2: start_addr=0xffff000008b1fb5c end_addr=0xffff000008b1fb6c

step 1: cs_etm__sample():
sample: ip=(0xffff000008b1fbfc-4) addr=0xffff000008b1fb5c

step 2: flush packet in cs_etm__run_decoder():
cs_etm__run_decoder()
`-> err = cs_etm__flush(etmq, false);
sample: ip=(0xffff000008b1fb6c-4) addr=0xffff000008b1fbf0

Packet1 and packet2 are two continuous packets, when packet2 is the new
coming packet, cs_etm__sample() generates branch sample for these two
packets and use [packet1::end_addr - 4 => packet2::start_addr] as branch
jump flow, thus we can see the first generated branch sample in step 1.
At the end of cs_etm__sample() it swaps packets so 'etm->prev_packet'=
packet2 and 'etm->packet'=packet1, so far it's okay for branch sample.

If packet2 is the last one packet in trace buffer, even there have no
any new coming packet, cs_etm__run_decoder() invokes cs_etm__flush() to
flush branch stack entries as expected, but it also generates branch
samples by taking 'etm->packet' as a new coming packet, thus the branch
jump flow is as [packet2::end_addr - 4 => packet1::start_addr]; this
is the second sample which is generated in step 2. So actually the
second sample is a stale sample and we should not generate it.

This patch introduces a new function cs_etm__end_block(), at the end of
trace block this function is invoked to only flush branch stack entries
and thus can avoid to generate branch sample for stale packet.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
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 789707b..ffc4fe5 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1055,6 +1055,39 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
return err;
}

+static int cs_etm__end_block(struct cs_etm_queue *etmq)
+{
+ int err;
+
+ /*
+ * It has no new packet coming and 'etmq->packet' contains the stale
+ * packet which was set at the previous time with packets swapping;
+ * so skip to generate branch sample to avoid stale packet.
+ *
+ * For this case only flush branch stack and generate a last branch
+ * event for the branches left in the circular buffer at the end of
+ * the trace.
+ */
+ if (etmq->etm->synth_opts.last_branch &&
+ etmq->prev_packet->sample_type == CS_ETM_RANGE) {
+ /*
+ * Use the address of the end of the last reported execution
+ * range.
+ */
+ u64 addr = cs_etm__last_executed_instr(etmq->prev_packet);
+
+ err = cs_etm__synth_instruction_sample(
+ etmq, addr,
+ etmq->period_instructions);
+ if (err)
+ return err;
+
+ etmq->period_instructions = 0;
+ }
+
+ return 0;
+}
+
static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
{
struct cs_etm_auxtrace *etm = etmq->etm;
@@ -1137,7 +1170,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)

if (err == 0)
/* Flush any remaining branch stack entries */
- err = cs_etm__flush(etmq);
+ err = cs_etm__end_block(etmq);
}

return err;
--
2.7.4


2018-12-11 07:42:10

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 3/8] perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder

cs_etm_decoder::trace_on is being assigned when TRACE_ON or NO_SYNC
element is coming, but it is never used hence it is redundant and can
be removed.

So let's remove 'trace_on' field from cs_etm_decoder struct.

Suggested-by: Mathieu Poirier <[email protected]>
Signed-off-by: Leo Yan <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 0b4c862..97b39b1 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -36,7 +36,6 @@
struct cs_etm_decoder {
void *data;
void (*packet_printer)(const char *msg);
- bool trace_on;
dcd_tree_handle_t dcd_tree;
cs_etm_mem_cb_type mem_access;
ocsd_datapath_resp_t prev_return;
@@ -411,12 +410,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
case OCSD_GEN_TRC_ELEM_NO_SYNC:
- decoder->trace_on = false;
break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_trace_on(decoder,
trace_chan_id);
- decoder->trace_on = true;
break;
case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
resp = cs_etm_decoder__buffer_range(decoder, elem,
--
2.7.4


2018-12-11 07:42:14

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 4/8] perf cs-etm: Refactor enumeration cs_etm_sample_type

The values in enumeration cs_etm_sample_type are defined with setting
bit N for each packet type, this is not suggested in the usual case.

This patch refactor cs_etm_sample_type by converting from bit shifting
values to continuous numbers.

Signed-off-by: Leo Yan <[email protected]>
Cc: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index b295dd2..3819a04 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -23,9 +23,9 @@ struct cs_etm_buffer {
};

enum cs_etm_sample_type {
- CS_ETM_EMPTY = 0,
- CS_ETM_RANGE = 1 << 0,
- CS_ETM_TRACE_ON = 1 << 1,
+ CS_ETM_EMPTY,
+ CS_ETM_RANGE,
+ CS_ETM_TRACE_ON,
};

enum cs_etm_isa {
--
2.7.4


2018-12-11 07:42:23

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 5/8] perf cs-etm: Rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY

TRACE_ON element is used at the beginning of trace, it also can be
appeared in the middle of trace data to indicate discontinuity; for
example, it's possible to see multiple TRACE_ON elements in the trace
stream if the trace is being limited by address range filtering.

Furthermore, except TRACE_ON element is for discontinuity, NO_SYNC and
EO_TRACE also can be used to indicate discontinuity, though they are
used for different scenarios for trace is interrupted.

This patch is to rename sample type CS_ETM_TRACE_ON to
CS_ETM_DISCONTINUITY, firstly the new name describes more closely the
purpose of the packet; secondly this is a preparation for other output
elements which also cause the trace discontinuity thus they can share
the same one packet type.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 +++++-----
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 2 +-
tools/perf/util/cs-etm.c | 12 ++++++------
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 97b39b1..1039f364 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -390,11 +390,11 @@ cs_etm_decoder__buffer_range(struct cs_etm_decoder *decoder,
}

static ocsd_datapath_resp_t
-cs_etm_decoder__buffer_trace_on(struct cs_etm_decoder *decoder,
- const uint8_t trace_chan_id)
+cs_etm_decoder__buffer_discontinuity(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
{
return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
- CS_ETM_TRACE_ON);
+ CS_ETM_DISCONTINUITY);
}

static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
@@ -412,8 +412,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_NO_SYNC:
break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
- resp = cs_etm_decoder__buffer_trace_on(decoder,
- trace_chan_id);
+ resp = cs_etm_decoder__buffer_discontinuity(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
resp = cs_etm_decoder__buffer_range(decoder, elem,
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index 3819a04..a272317 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -25,7 +25,7 @@ struct cs_etm_buffer {
enum cs_etm_sample_type {
CS_ETM_EMPTY,
CS_ETM_RANGE,
- CS_ETM_TRACE_ON,
+ CS_ETM_DISCONTINUITY,
};

enum cs_etm_isa {
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ffc4fe5..cea3158 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -562,8 +562,8 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,

static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
{
- /* Returns 0 for the CS_ETM_TRACE_ON packet */
- if (packet->sample_type == CS_ETM_TRACE_ON)
+ /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
+ if (packet->sample_type == CS_ETM_DISCONTINUITY)
return 0;

return packet->start_addr;
@@ -572,8 +572,8 @@ static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
static inline
u64 cs_etm__last_executed_instr(const struct cs_etm_packet *packet)
{
- /* Returns 0 for the CS_ETM_TRACE_ON packet */
- if (packet->sample_type == CS_ETM_TRACE_ON)
+ /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
+ if (packet->sample_type == CS_ETM_DISCONTINUITY)
return 0;

return packet->end_addr - packet->last_instr_size;
@@ -972,7 +972,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
bool generate_sample = false;

/* Generate sample for tracing on packet */
- if (etmq->prev_packet->sample_type == CS_ETM_TRACE_ON)
+ if (etmq->prev_packet->sample_type == CS_ETM_DISCONTINUITY)
generate_sample = true;

/* Generate sample for branch taken packet */
@@ -1148,7 +1148,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
*/
cs_etm__sample(etmq);
break;
- case CS_ETM_TRACE_ON:
+ case CS_ETM_DISCONTINUITY:
/*
* Discontinuity in trace, flush
* previous branch stack
--
2.7.4


2018-12-11 07:42:40

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

CoreSight tracer driver might insert barrier packet between different
buffers, thus the decoder can spot the boundaries based on the barrier
packet; the decoder is possible to hit a barrier packet and emit a
NO_SYNC element, then the decoder will find a periodic synchronisation
point inside that next trace block that starts trace again but does not
have the TRACE_ON element as indicator - usually because this block of
trace has wrapped the buffer so we have lost the original point that
trace was enabled.

In upper case, it results in the trace stream only inserts the
OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
we don't handle NO_SYNC element properly and at the end users miss to
see the info for trace discontinuity.

Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
output from the decoder, but both of them indicate the trace data is
discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
cs-etm can handle discontinuity for this case, finally it saves the last
trace data for previous trace block and restart samples for new block.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 1039f364..bee026e 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
case OCSD_GEN_TRC_ELEM_NO_SYNC:
- break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
trace_chan_id);
--
2.7.4


2018-12-11 07:42:54

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 7/8] perf cs-etm: Treat EO_TRACE element as trace discontinuity

If decoder outputs EO_TRACE element, it means the end of the trace
buffer; this is a discontinuity and in this case the end of trace data
needs to be saved.

This patch generates CS_ETM_DISCONTINUITY packet for EO_TRACE element
hereby flushing the end of trace data in cs-etm.c.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index bee026e..cda6f07 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -409,6 +409,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
switch (elem->elem_type) {
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
+ case OCSD_GEN_TRC_ELEM_EO_TRACE:
case OCSD_GEN_TRC_ELEM_NO_SYNC:
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
@@ -425,7 +426,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
decoder->packet_buffer[decoder->tail].exc_ret = true;
break;
case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
- case OCSD_GEN_TRC_ELEM_EO_TRACE:
case OCSD_GEN_TRC_ELEM_ADDR_NACC:
case OCSD_GEN_TRC_ELEM_TIMESTAMP:
case OCSD_GEN_TRC_ELEM_CYCLE_COUNT:
--
2.7.4


2018-12-11 07:44:00

by Leo Yan

[permalink] [raw]
Subject: [PATCH v3 8/8] perf cs-etm: Generate branch sample for exception packet

The exception packet appears as one element with 'elem_type' ==
OCSD_GEN_TRC_ELEM_EXCEPTION or OCSD_GEN_TRC_ELEM_EXCEPTION_RET,
which present for exception entry and exit respectively. The decoder
set packet fields 'packet->exc' and 'packet->exc_ret' to indicate the
exception packets; but exception packets don't have dedicated sample
type and shares the same sample type CS_ETM_RANGE with normal
instruction packets.

As result, the exception packets are taken as normal instruction packets
and this introduces confusion to mix different packet types.
Furthermore, these instruction range packets will be processed for
branch sample only when 'packet->last_instr_taken_branch' is true,
otherwise they will be omitted, this can introduce mess for exception
and exception returning due we don't have complete address range info
for context switching.

To process exception packets properly, this patch introduce two new
sample type: CS_ETM_EXCEPTION and CS_ETM_EXCEPTION_RET; for these two
kind packets, they will be handled by cs_etm__exception(). The func
cs_etm__exception() forces to set previous CS_ETM_RANGE packet flag
'prev_packet->last_instr_taken_branch' to true, this matches well with
the program flow when the exception is trapped from user space to kernel
space, no matter if the most recent flow has branch taken or not; this
is also safe for returning to user space after exception handling.

After exception packets have their own sample type, the packet fields
'packet->exc' and 'packet->exc_ret' aren't needed anymore, so remove
them.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Robert Walker <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 26 +++++++++++++++++------
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 4 ++--
tools/perf/util/cs-etm.c | 28 +++++++++++++++++++++++++
3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index cda6f07..8c15557 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -290,8 +290,6 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder)
decoder->packet_buffer[i].instr_count = 0;
decoder->packet_buffer[i].last_instr_taken_branch = false;
decoder->packet_buffer[i].last_instr_size = 0;
- decoder->packet_buffer[i].exc = false;
- decoder->packet_buffer[i].exc_ret = false;
decoder->packet_buffer[i].cpu = INT_MIN;
}
}
@@ -319,8 +317,6 @@ cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder,

decoder->packet_buffer[et].sample_type = sample_type;
decoder->packet_buffer[et].isa = CS_ETM_ISA_UNKNOWN;
- decoder->packet_buffer[et].exc = false;
- decoder->packet_buffer[et].exc_ret = false;
decoder->packet_buffer[et].cpu = *((int *)inode->priv);
decoder->packet_buffer[et].start_addr = CS_ETM_INVAL_ADDR;
decoder->packet_buffer[et].end_addr = CS_ETM_INVAL_ADDR;
@@ -397,6 +393,22 @@ cs_etm_decoder__buffer_discontinuity(struct cs_etm_decoder *decoder,
CS_ETM_DISCONTINUITY);
}

+static ocsd_datapath_resp_t
+cs_etm_decoder__buffer_exception(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
+{
+ return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
+ CS_ETM_EXCEPTION);
+}
+
+static ocsd_datapath_resp_t
+cs_etm_decoder__buffer_exception_ret(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
+{
+ return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
+ CS_ETM_EXCEPTION_RET);
+}
+
static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
const void *context,
const ocsd_trc_index_t indx __maybe_unused,
@@ -420,10 +432,12 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_EXCEPTION:
- decoder->packet_buffer[decoder->tail].exc = true;
+ resp = cs_etm_decoder__buffer_exception(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_EXCEPTION_RET:
- decoder->packet_buffer[decoder->tail].exc_ret = true;
+ resp = cs_etm_decoder__buffer_exception_ret(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
case OCSD_GEN_TRC_ELEM_ADDR_NACC:
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index a272317..a6407d4 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -26,6 +26,8 @@ enum cs_etm_sample_type {
CS_ETM_EMPTY,
CS_ETM_RANGE,
CS_ETM_DISCONTINUITY,
+ CS_ETM_EXCEPTION,
+ CS_ETM_EXCEPTION_RET,
};

enum cs_etm_isa {
@@ -43,8 +45,6 @@ struct cs_etm_packet {
u32 instr_count;
u8 last_instr_taken_branch;
u8 last_instr_size;
- u8 exc;
- u8 exc_ret;
int cpu;
};

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index cea3158..27a374d 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1000,6 +1000,25 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
return 0;
}

+static int cs_etm__exception(struct cs_etm_queue *etmq)
+{
+ /*
+ * When the exception packet is inserted, whether the last instruction
+ * in previous range packet is taken branch or not, we need to force
+ * to set 'prev_packet->last_instr_taken_branch' to true. This ensures
+ * to generate branch sample for the instruction range before the
+ * exception is trapped to kernel or before the exception returning.
+ *
+ * The exception packet includes the dummy address values, so don't
+ * swap PACKET with PREV_PACKET. This keeps PREV_PACKET to be useful
+ * for generating instruction and branch samples.
+ */
+ if (etmq->prev_packet->sample_type == CS_ETM_RANGE)
+ etmq->prev_packet->last_instr_taken_branch = true;
+
+ return 0;
+}
+
static int cs_etm__flush(struct cs_etm_queue *etmq)
{
int err = 0;
@@ -1148,6 +1167,15 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
*/
cs_etm__sample(etmq);
break;
+ case CS_ETM_EXCEPTION:
+ case CS_ETM_EXCEPTION_RET:
+ /*
+ * If the exception packet is coming,
+ * make sure the previous instruction
+ * range packet to be handled properly.
+ */
+ cs_etm__exception(etmq);
+ break;
case CS_ETM_DISCONTINUITY:
/*
* Discontinuity in trace, flush
--
2.7.4


2018-12-12 18:42:25

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 3/8] perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder

On Tue, Dec 11, 2018 at 03:38:23PM +0800, Leo Yan wrote:
> cs_etm_decoder::trace_on is being assigned when TRACE_ON or NO_SYNC
> element is coming, but it is never used hence it is redundant and can
> be removed.
>
> So let's remove 'trace_on' field from cs_etm_decoder struct.
>
> Suggested-by: Mathieu Poirier <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Robert Walker <[email protected]>
> ---
> tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 0b4c862..97b39b1 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -36,7 +36,6 @@
> struct cs_etm_decoder {
> void *data;
> void (*packet_printer)(const char *msg);
> - bool trace_on;
> dcd_tree_handle_t dcd_tree;
> cs_etm_mem_cb_type mem_access;
> ocsd_datapath_resp_t prev_return;
> @@ -411,12 +410,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
> case OCSD_GEN_TRC_ELEM_UNKNOWN:
> break;
> case OCSD_GEN_TRC_ELEM_NO_SYNC:
> - decoder->trace_on = false;
> break;
> case OCSD_GEN_TRC_ELEM_TRACE_ON:
> resp = cs_etm_decoder__buffer_trace_on(decoder,
> trace_chan_id);
> - decoder->trace_on = true;
> break;
> case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
> resp = cs_etm_decoder__buffer_range(decoder, elem,

Reviewed-by: Mathieu Poirier <[email protected]>

> --
> 2.7.4
>

2018-12-12 18:44:39

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 4/8] perf cs-etm: Refactor enumeration cs_etm_sample_type

On Tue, Dec 11, 2018 at 03:38:24PM +0800, Leo Yan wrote:
> The values in enumeration cs_etm_sample_type are defined with setting
> bit N for each packet type, this is not suggested in the usual case.
>
> This patch refactor cs_etm_sample_type by converting from bit shifting
> values to continuous numbers.
>
> Signed-off-by: Leo Yan <[email protected]>
> Cc: Mathieu Poirier <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Robert Walker <[email protected]>
> ---
> tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
> index b295dd2..3819a04 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
> @@ -23,9 +23,9 @@ struct cs_etm_buffer {
> };
>
> enum cs_etm_sample_type {
> - CS_ETM_EMPTY = 0,
> - CS_ETM_RANGE = 1 << 0,
> - CS_ETM_TRACE_ON = 1 << 1,
> + CS_ETM_EMPTY,
> + CS_ETM_RANGE,
> + CS_ETM_TRACE_ON,
> };

Reviewed-by: Mathieu Poirier <[email protected]>

>
> enum cs_etm_isa {
> --
> 2.7.4
>

2018-12-12 18:47:10

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] perf cs-etm: Correct packets handling

On Tue, 11 Dec 2018 at 00:40, Leo Yan <[email protected]> wrote:
>
> perf cs-etm module converts decoder elements to packets and then we have
> more context crossing packets to generate synthenize samples, finally
> perf tool can faciliate samples for statistics and report the results.
>
> This patch series is to address several issues found related with
> packets handling and samples generation when worked firstly on branch
> sample flags support for Arm CoreSight trace data, so this patch series
> is dependency for sample flags setting, will send another dedicated
> patch series for sample flags later.
>
> In this patch series, the first two patches are mainly to fix issues in
> cs_etm__flush(): patch 0001 corrects packets swapping in cs_etm__flush()
> and this can fix the wrong branch sample caused by the missed packets
> swapping; patch 0002 is to fix the wrong samples generation with stale
> packets at the end of trace block.
>
> Patch 0003 and 0004 are for minor fixing; patch 0003 removes unused field
> 'cs_etm_decoder::trace_on', this can simplize the switch-case code for all
> discontinuity packet generation by using one code block; patch 0004 is to
> refactor enumeration cs_etm_sample_type.
>
> Patch 0005 is to rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY, we use
> a more general packet type to present trace discontinuity, so it can be
> used by TRACE_ON event, and also can be used by NO_SYNC and EO_TRACE
> elements.
>
> Patch 0006 is used to support NO_SYNC packet, otherwise the trace
> decoding cannot reflect the tracing discontinuity caused by NO_SYNC
> packet.
>
> Patch 0007 is used to support EO_TRACE packet, which also introduces
> the tracing discontinuity at the end of trace and we should save last
> trace data for it.
>
> Patch 0008 is used to generate branch sample for exception packets.
>
> Credit to Mike Leach and Robert Walker who made me clear for underlying
> mechanism for NO_SYNC/EO_TRACE elements, Mike also shared the detailed
> explanation for why we can treat NO_SYNC and TRACE_ON elements as the
> same, so except following Mike & Rob suggestion for trace discontinuity
> consolidation, most commit log of patches 0006/0007 also come from
> Mike's explanation.
>
> This patch series is applied directly on the acme's perf/core branch [1]
> with latest commit aaab25f03e9e ("perf trace: Allow selecting use the
> use of the ordered_events code").
>
> With applying the dependency patch, this patch series has been tested
> for branch samples dumping with below command on Juno board:
>
> # perf script -F,-time,+ip,+sym,+dso,+addr,+symoff -k vmlinux

Good day Arnaldo,

I have reviewed all the changes in this set - please consider adding
to your next branch. Let me know if you want a pull request.

Regards,
Mathieu

>
> Changes from v2:
> * Addressed Mathieu's comments and suggestions for minor refactoring
> for removing unused field 'cs_etm_decoder::trace_on' and added one
> dedicated patch to refactor enumeration cs_etm_sample_type.
> * Added Mathieu's 'reviewed' tags; Very appreciate Mathieu's many
> suggestion for crossing several patch series.
>
> Changes from v1:
> * Synced the consistent code in patch 0001 for condition checking.
> * Introduced new function cs_etm__end_block() for flushing packet
> at the end of trace block.
> * Added new patch 0003 to rename CS_ETM_TRACE_ON to
> CS_ETM_DISCONTINUITY.
> * Used the same one packet type CS_ETM_DISCONTINUITY for all
> trace discontinuity (include support TRACE_ON/EO_TRACE/NO_SYNC
> packets).
> * Removed tracking exception number patch, which will be added in
> sample flag patch series.
>
>
> Leo Yan (8):
> perf cs-etm: Correct packets swapping in cs_etm__flush()
> perf cs-etm: Avoid stale branch samples when flush packet
> perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder
> perf cs-etm: Refactor enumeration cs_etm_sample_type
> perf cs-etm: Rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY
> perf cs-etm: Treat NO_SYNC element as trace discontinuity
> perf cs-etm: Treat EO_TRACE element as trace discontinuity
> perf cs-etm: Generate branch sample for exception packet
>
> tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 42 +++++++++-----
> tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 10 ++--
> tools/perf/util/cs-etm.c | 77 ++++++++++++++++++++++---
> 3 files changed, 100 insertions(+), 29 deletions(-)
>
> --
> 2.7.4
>

2018-12-13 12:40:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu:
> CoreSight tracer driver might insert barrier packet between different
> buffers, thus the decoder can spot the boundaries based on the barrier
> packet; the decoder is possible to hit a barrier packet and emit a
> NO_SYNC element, then the decoder will find a periodic synchronisation
> point inside that next trace block that starts trace again but does not
> have the TRACE_ON element as indicator - usually because this block of
> trace has wrapped the buffer so we have lost the original point that
> trace was enabled.
>
> In upper case, it results in the trace stream only inserts the
> OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
> we don't handle NO_SYNC element properly and at the end users miss to
> see the info for trace discontinuity.


"In upper case"? Maybe:

In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
in the middle of the the tracing stream, but as we were npt handling the
NO_SYNC element properly which ends up making users miss the
discontinuity indication"?


> Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> output from the decoder, but both of them indicate the trace data is

can we remove the "but" and "of them" (redundant) above?

> discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
a
> discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
> cs-etm can handle discontinuity for this case, finally it saves the last
it (way too many "discontinuity")
> trace data for previous trace block and restart samples for new block.
>
> Signed-off-by: Leo Yan <[email protected]>
> Reviewed-by: Mathieu Poirier <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Robert Walker <[email protected]>
> ---
> tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 1039f364..bee026e 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
> case OCSD_GEN_TRC_ELEM_UNKNOWN:
> break;
> case OCSD_GEN_TRC_ELEM_NO_SYNC:
> - break;
> case OCSD_GEN_TRC_ELEM_TRACE_ON:
> resp = cs_etm_decoder__buffer_discontinuity(decoder,
> trace_chan_id);
> --
> 2.7.4

--

- Arnaldo

2018-12-13 12:43:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu:
> > CoreSight tracer driver might insert barrier packet between different
> > buffers, thus the decoder can spot the boundaries based on the barrier
> > packet; the decoder is possible to hit a barrier packet and emit a
> > NO_SYNC element, then the decoder will find a periodic synchronisation
> > point inside that next trace block that starts trace again but does not
> > have the TRACE_ON element as indicator - usually because this block of
> > trace has wrapped the buffer so we have lost the original point that
> > trace was enabled.
> >
> > In upper case, it results in the trace stream only inserts the
> > OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
> > we don't handle NO_SYNC element properly and at the end users miss to
> > see the info for trace discontinuity.
>
>
> "In upper case"? Maybe:
>
> In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
> in the middle of the the tracing stream, but as we were npt handling the
> NO_SYNC element properly which ends up making users miss the
> discontinuity indication"?

> > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> > output from the decoder, but both of them indicate the trace data is
>
> can we remove the "but" and "of them" (redundant) above?
>
> > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
> a
> > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
> > cs-etm can handle discontinuity for this case, finally it saves the last
> it (way too many "discontinuity")
> > trace data for previous trace block and restart samples for new block.

I've tentatively done those changes (and a few more) in my local branch,
resulting in the wording below, plese let me know if you are ok with it:


commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59
Author: Leo Yan <[email protected]>
Date: Tue Dec 11 15:38:26 2018 +0800

perf cs-etm: Treat NO_SYNC element as trace discontinuity

The CoreSight tracer driver might insert a barrier packets between
different buffers, thus the decoder can spot the boundaries based on the
barrier packet; it is possible for the decoder to hit a barrier packet
and emit a NO_SYNC element, then the decoder will find a periodic
synchronisation point inside that next trace block that starts the trace
again but does not have the TRACE_ON element as indicator - usually
because this trace block has wrapped the buffer so we have lost the
original point when the trace was enabled.

In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
in the middle of the the tracing stream, but as we were npt handling the
NO_SYNC element properly which ends up making users miss the
discontinuity indication"?

Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
output from the decoder, both indicate that the trace data is
discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace
discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so
cs-etm can handle the discontinuity for this case, finally it saves the
last trace data for the previous trace block and restart samples for the
new block.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: coresight ml <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 1039f364f4cc..bee026e76a4c 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
case OCSD_GEN_TRC_ELEM_NO_SYNC:
- break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
trace_chan_id);



2018-12-13 13:12:00

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Hi Arnaldo,

On Thu, Dec 13, 2018 at 09:41:54AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu:
> > > CoreSight tracer driver might insert barrier packet between different
> > > buffers, thus the decoder can spot the boundaries based on the barrier
> > > packet; the decoder is possible to hit a barrier packet and emit a
> > > NO_SYNC element, then the decoder will find a periodic synchronisation
> > > point inside that next trace block that starts trace again but does not
> > > have the TRACE_ON element as indicator - usually because this block of
> > > trace has wrapped the buffer so we have lost the original point that
> > > trace was enabled.
> > >
> > > In upper case, it results in the trace stream only inserts the
> > > OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
> > > we don't handle NO_SYNC element properly and at the end users miss to
> > > see the info for trace discontinuity.
> >
> >
> > "In upper case"? Maybe:
> >
> > In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
> > in the middle of the the tracing stream, but as we were npt handling the
> > NO_SYNC element properly which ends up making users miss the
> > discontinuity indication"?
>
> > > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> > > output from the decoder, but both of them indicate the trace data is
> >
> > can we remove the "but" and "of them" (redundant) above?
> >
> > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
> > a
> > > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
> > > cs-etm can handle discontinuity for this case, finally it saves the last
> > it (way too many "discontinuity")
> > > trace data for previous trace block and restart samples for new block.
>
> I've tentatively done those changes (and a few more) in my local branch,
> resulting in the wording below, plese let me know if you are ok with it:

Very appreciate your helping. Please see some minor typos in below
commit:

> commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59
> Author: Leo Yan <[email protected]>
> Date: Tue Dec 11 15:38:26 2018 +0800
>
> perf cs-etm: Treat NO_SYNC element as trace discontinuity
>
> The CoreSight tracer driver might insert a barrier packets between
packet
> different buffers, thus the decoder can spot the boundaries based on the
> barrier packet; it is possible for the decoder to hit a barrier packet
> and emit a NO_SYNC element, then the decoder will find a periodic
> synchronisation point inside that next trace block that starts the trace
> again but does not have the TRACE_ON element as indicator - usually
> because this trace block has wrapped the buffer so we have lost the
> original point when the trace was enabled.
>
> In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
former
> in the middle of the the tracing stream, but as we were npt handling the
not
> NO_SYNC element properly which ends up making users miss the
> discontinuity indication"?
s/?/.

Thanks a lot for helping. If you prefer me to resend the patch set,
also let me know.

> Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> output from the decoder, both indicate that the trace data is
> discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace
> discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so
> cs-etm can handle the discontinuity for this case, finally it saves the
> last trace data for the previous trace block and restart samples for the
> new block.
>
> Signed-off-by: Leo Yan <[email protected]>
> Reviewed-by: Mathieu Poirier <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Robert Walker <[email protected]>
> Cc: coresight ml <[email protected]>
> Cc: [email protected]
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 1039f364f4cc..bee026e76a4c 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
> case OCSD_GEN_TRC_ELEM_UNKNOWN:
> break;
> case OCSD_GEN_TRC_ELEM_NO_SYNC:
> - break;
> case OCSD_GEN_TRC_ELEM_TRACE_ON:
> resp = cs_etm_decoder__buffer_discontinuity(decoder,
> trace_chan_id);
>
>

2018-12-13 13:14:01

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] perf cs-etm: Correct packets handling

Em Wed, Dec 12, 2018 at 11:45:44AM -0700, Mathieu Poirier escreveu:
> On Tue, 11 Dec 2018 at 00:40, Leo Yan <[email protected]> wrote:
> >
> > perf cs-etm module converts decoder elements to packets and then we have
> > more context crossing packets to generate synthenize samples, finally
> > perf tool can faciliate samples for statistics and report the results.
> >
> > This patch series is to address several issues found related with
> > packets handling and samples generation when worked firstly on branch
> > sample flags support for Arm CoreSight trace data, so this patch series
> > is dependency for sample flags setting, will send another dedicated
> > patch series for sample flags later.
> >
> > In this patch series, the first two patches are mainly to fix issues in
> > cs_etm__flush(): patch 0001 corrects packets swapping in cs_etm__flush()
> > and this can fix the wrong branch sample caused by the missed packets
> > swapping; patch 0002 is to fix the wrong samples generation with stale
> > packets at the end of trace block.
> >
> > Patch 0003 and 0004 are for minor fixing; patch 0003 removes unused field
> > 'cs_etm_decoder::trace_on', this can simplize the switch-case code for all
> > discontinuity packet generation by using one code block; patch 0004 is to
> > refactor enumeration cs_etm_sample_type.
> >
> > Patch 0005 is to rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY, we use
> > a more general packet type to present trace discontinuity, so it can be
> > used by TRACE_ON event, and also can be used by NO_SYNC and EO_TRACE
> > elements.
> >
> > Patch 0006 is used to support NO_SYNC packet, otherwise the trace
> > decoding cannot reflect the tracing discontinuity caused by NO_SYNC
> > packet.
> >
> > Patch 0007 is used to support EO_TRACE packet, which also introduces
> > the tracing discontinuity at the end of trace and we should save last
> > trace data for it.
> >
> > Patch 0008 is used to generate branch sample for exception packets.
> >
> > Credit to Mike Leach and Robert Walker who made me clear for underlying
> > mechanism for NO_SYNC/EO_TRACE elements, Mike also shared the detailed
> > explanation for why we can treat NO_SYNC and TRACE_ON elements as the
> > same, so except following Mike & Rob suggestion for trace discontinuity
> > consolidation, most commit log of patches 0006/0007 also come from
> > Mike's explanation.
> >
> > This patch series is applied directly on the acme's perf/core branch [1]
> > with latest commit aaab25f03e9e ("perf trace: Allow selecting use the
> > use of the ordered_events code").
> >
> > With applying the dependency patch, this patch series has been tested
> > for branch samples dumping with below command on Juno board:
> >
> > # perf script -F,-time,+ip,+sym,+dso,+addr,+symoff -k vmlinux
>
> Good day Arnaldo,
>
> I have reviewed all the changes in this set - please consider adding
> to your next branch. Let me know if you want a pull request.

Thanks, applied manually, pushing now to my tmp.perf/core branch, please
take a look at the commit log messages to see if I didn't messed up
anything. :-)

- Arnaldo

2018-12-13 13:23:14

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Em Thu, Dec 13, 2018 at 09:09:49PM +0800, [email protected] escreveu:
> Hi Arnaldo,
>
> On Thu, Dec 13, 2018 at 09:41:54AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu:
> > > > CoreSight tracer driver might insert barrier packet between different
> > > > buffers, thus the decoder can spot the boundaries based on the barrier
> > > > packet; the decoder is possible to hit a barrier packet and emit a
> > > > NO_SYNC element, then the decoder will find a periodic synchronisation
> > > > point inside that next trace block that starts trace again but does not
> > > > have the TRACE_ON element as indicator - usually because this block of
> > > > trace has wrapped the buffer so we have lost the original point that
> > > > trace was enabled.
> > > >
> > > > In upper case, it results in the trace stream only inserts the
> > > > OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
> > > > we don't handle NO_SYNC element properly and at the end users miss to
> > > > see the info for trace discontinuity.
> > >
> > >
> > > "In upper case"? Maybe:
> > >
> > > In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
> > > in the middle of the the tracing stream, but as we were npt handling the
> > > NO_SYNC element properly which ends up making users miss the
> > > discontinuity indication"?
> >
> > > > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> > > > output from the decoder, but both of them indicate the trace data is
> > >
> > > can we remove the "but" and "of them" (redundant) above?
> > >
> > > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
> > > a
> > > > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
> > > > cs-etm can handle discontinuity for this case, finally it saves the last
> > > it (way too many "discontinuity")
> > > > trace data for previous trace block and restart samples for new block.
> >
> > I've tentatively done those changes (and a few more) in my local branch,
> > resulting in the wording below, plese let me know if you are ok with it:
>
> Very appreciate your helping. Please see some minor typos in below
> commit:
>
> > commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59
> > Author: Leo Yan <[email protected]>
> > Date: Tue Dec 11 15:38:26 2018 +0800
> >
> > perf cs-etm: Treat NO_SYNC element as trace discontinuity
> >
> > The CoreSight tracer driver might insert a barrier packets between
> packet

I'll remove the 'a' instead, ok?


> > different buffers, thus the decoder can spot the boundaries based on the
> > barrier packet; it is possible for the decoder to hit a barrier packet
> > and emit a NO_SYNC element, then the decoder will find a periodic
> > synchronisation point inside that next trace block that starts the trace
> > again but does not have the TRACE_ON element as indicator - usually
> > because this trace block has wrapped the buffer so we have lost the
> > original point when the trace was enabled.
> >
> > In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
> former
> > in the middle of the the tracing stream, but as we were npt handling the
> not
> > NO_SYNC element properly which ends up making users miss the
> > discontinuity indication"?
> s/?/.
>
> Thanks a lot for helping. If you prefer me to resend the patch set,
> also let me know.

I can fix it, here.

> > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> > output from the decoder, both indicate that the trace data is
> > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace
> > discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so
> > cs-etm can handle the discontinuity for this case, finally it saves the
> > last trace data for the previous trace block and restart samples for the
> > new block.
> >
> > Signed-off-by: Leo Yan <[email protected]>
> > Reviewed-by: Mathieu Poirier <[email protected]>
> > Cc: Alexander Shishkin <[email protected]>
> > Cc: Jiri Olsa <[email protected]>
> > Cc: Mike Leach <[email protected]>
> > Cc: Namhyung Kim <[email protected]>
> > Cc: Robert Walker <[email protected]>
> > Cc: coresight ml <[email protected]>
> > Cc: [email protected]
> > Link: http://lkml.kernel.org/r/[email protected]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> >
> > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > index 1039f364f4cc..bee026e76a4c 100644
> > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > @@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
> > case OCSD_GEN_TRC_ELEM_UNKNOWN:
> > break;
> > case OCSD_GEN_TRC_ELEM_NO_SYNC:
> > - break;
> > case OCSD_GEN_TRC_ELEM_TRACE_ON:
> > resp = cs_etm_decoder__buffer_discontinuity(decoder,
> > trace_chan_id);
> >
> >

--

- Arnaldo

2018-12-13 13:26:03

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

On Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> > > commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59
> > > Author: Leo Yan <[email protected]>
> > > Date: Tue Dec 11 15:38:26 2018 +0800
> > >
> > > perf cs-etm: Treat NO_SYNC element as trace discontinuity
> > >
> > > The CoreSight tracer driver might insert a barrier packets between
> > packet
>
> I'll remove the 'a' instead, ok?

Yeah.

> > > different buffers, thus the decoder can spot the boundaries based on the
> > > barrier packet; it is possible for the decoder to hit a barrier packet
> > > and emit a NO_SYNC element, then the decoder will find a periodic
> > > synchronisation point inside that next trace block that starts the trace
> > > again but does not have the TRACE_ON element as indicator - usually
> > > because this trace block has wrapped the buffer so we have lost the
> > > original point when the trace was enabled.
> > >
> > > In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
> > former
> > > in the middle of the the tracing stream, but as we were npt handling the
> > not
> > > NO_SYNC element properly which ends up making users miss the
> > > discontinuity indication"?
> > s/?/.
> >
> > Thanks a lot for helping. If you prefer me to resend the patch set,
> > also let me know.
>
> I can fix it, here.

Thanks a lot!

> > > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> > > output from the decoder, both indicate that the trace data is
> > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace
> > > discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so
> > > cs-etm can handle the discontinuity for this case, finally it saves the
> > > last trace data for the previous trace block and restart samples for the
> > > new block.
> > >
> > > Signed-off-by: Leo Yan <[email protected]>
> > > Reviewed-by: Mathieu Poirier <[email protected]>
> > > Cc: Alexander Shishkin <[email protected]>
> > > Cc: Jiri Olsa <[email protected]>
> > > Cc: Mike Leach <[email protected]>
> > > Cc: Namhyung Kim <[email protected]>
> > > Cc: Robert Walker <[email protected]>
> > > Cc: coresight ml <[email protected]>
> > > Cc: [email protected]
> > > Link: http://lkml.kernel.org/r/[email protected]
> > > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> > >
> > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > index 1039f364f4cc..bee026e76a4c 100644
> > > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > @@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
> > > case OCSD_GEN_TRC_ELEM_UNKNOWN:
> > > break;
> > > case OCSD_GEN_TRC_ELEM_NO_SYNC:
> > > - break;
> > > case OCSD_GEN_TRC_ELEM_TRACE_ON:
> > > resp = cs_etm_decoder__buffer_discontinuity(decoder,
> > > trace_chan_id);
> > >
> > >
>
> --
>
> - Arnaldo

2018-12-13 13:29:19

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Em Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Dec 13, 2018 at 09:09:49PM +0800, [email protected] escreveu:
> > Thanks a lot for helping. If you prefer me to resend the patch set,
> > also let me know.
>
> I can fix it, here.

Can I take a look at the result:

https://git.kernel.org/acme/c/222b0bead7e4

?

- Arnaldo

2018-12-13 13:35:13

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity

On Thu, Dec 13, 2018 at 10:26:33AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Dec 13, 2018 at 09:09:49PM +0800, [email protected] escreveu:
> > > Thanks a lot for helping. If you prefer me to resend the patch set,
> > > also let me know.
> >
> > I can fix it, here.
>
> Can I take a look at the result:
>
> https://git.kernel.org/acme/c/222b0bead7e4

Yeah, looks good to me :)

Thanks,
Leo Yan

Subject: [tip:perf/core] perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder

Commit-ID: cee7a6a212ddb514d6e2ac1f9733501c43d63d0a
Gitweb: https://git.kernel.org/tip/cee7a6a212ddb514d6e2ac1f9733501c43d63d0a
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:23 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Remove unused 'trace_on' in cs_etm_decoder

cs_etm_decoder::trace_on is being assigned when TRACE_ON or NO_SYNC
element is coming, but it is never used hence it is redundant and can
be removed.

So let's remove 'trace_on' field from cs_etm_decoder struct.

Suggested-by: Mathieu Poirier <[email protected]>
Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 0b4c8629f578..97b39b1f5759 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -36,7 +36,6 @@
struct cs_etm_decoder {
void *data;
void (*packet_printer)(const char *msg);
- bool trace_on;
dcd_tree_handle_t dcd_tree;
cs_etm_mem_cb_type mem_access;
ocsd_datapath_resp_t prev_return;
@@ -411,12 +410,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
case OCSD_GEN_TRC_ELEM_NO_SYNC:
- decoder->trace_on = false;
break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_trace_on(decoder,
trace_chan_id);
- decoder->trace_on = true;
break;
case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
resp = cs_etm_decoder__buffer_range(decoder, elem,

Subject: [tip:perf/core] perf cs-etm: Correct packets swapping in cs_etm__flush()

Commit-ID: 43fd56669c28cd354e9228bdb58e4bca1c1a8b66
Gitweb: https://git.kernel.org/tip/43fd56669c28cd354e9228bdb58e4bca1c1a8b66
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:21 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:58 -0300

perf cs-etm: Correct packets swapping in cs_etm__flush()

The structure cs_etm_queue uses 'prev_packet' to point to previous
packet, this can be used to combine with new coming packet to generate
samples.

In function cs_etm__flush() it swaps packets only when the flag
'etm->synth_opts.last_branch' is true, this means that it will not swap
packets if without option '--itrace=il' to generate last branch entries;
thus for this case the 'prev_packet' doesn't point to the correct
previous packet and the stale packet still will be used to generate
sequential sample. Thus if dump trace with 'perf script' command we can
see the incorrect flow with the stale packet's address info.

This patch corrects packets swapping in cs_etm__flush(); except using
the flag 'etm->synth_opts.last_branch' it also checks the another flag
'etm->sample_branches', if any flag is true then it swaps packets so can
save correct content to 'prev_packet'. Finally this can fix the wrong
program flow dumping issue.

The patch has a minor refactoring to use 'etm->synth_opts.last_branch'
instead of 'etmq->etm->synth_opts.last_branch' for condition checking,
this is consistent with that is done in cs_etm__sample().

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 23159c33db2a..789707bc4e9e 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1042,7 +1042,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
}

swap_packet:
- if (etmq->etm->synth_opts.last_branch) {
+ if (etm->sample_branches || etm->synth_opts.last_branch) {
/*
* Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
* the next incoming packet.

Subject: [tip:perf/core] perf cs-etm: Avoid stale branch samples when flush packet

Commit-ID: 24fff5eb2b937951af5dce6cdb7a1e679ac8e751
Gitweb: https://git.kernel.org/tip/24fff5eb2b937951af5dce6cdb7a1e679ac8e751
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:22 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Avoid stale branch samples when flush packet

At the end of trace buffer handling, function cs_etm__flush() is invoked
to flush any remaining branch stack entries. As a side effect, it also
generates branch sample, because the 'etmq->packet' doesn't contains any
new coming packet but point to one stale packet after packets swapping,
so it wrongly makes synthesize branch samples with stale packet info.

We could review below detailed flow which causes issue:

Packet1: start_addr=0xffff000008b1fbf0 end_addr=0xffff000008b1fbfc
Packet2: start_addr=0xffff000008b1fb5c end_addr=0xffff000008b1fb6c

step 1: cs_etm__sample():
sample: ip=(0xffff000008b1fbfc-4) addr=0xffff000008b1fb5c

step 2: flush packet in cs_etm__run_decoder():
cs_etm__run_decoder()
`-> err = cs_etm__flush(etmq, false);
sample: ip=(0xffff000008b1fb6c-4) addr=0xffff000008b1fbf0

Packet1 and packet2 are two continuous packets, when packet2 is the new
coming packet, cs_etm__sample() generates branch sample for these two
packets and use [packet1::end_addr - 4 => packet2::start_addr] as branch
jump flow, thus we can see the first generated branch sample in step 1.
At the end of cs_etm__sample() it swaps packets so 'etm->prev_packet'=
packet2 and 'etm->packet'=packet1, so far it's okay for branch sample.

If packet2 is the last one packet in trace buffer, even there have no
any new coming packet, cs_etm__run_decoder() invokes cs_etm__flush() to
flush branch stack entries as expected, but it also generates branch
samples by taking 'etm->packet' as a new coming packet, thus the branch
jump flow is as [packet2::end_addr - 4 => packet1::start_addr]; this
is the second sample which is generated in step 2. So actually the
second sample is a stale sample and we should not generate it.

This patch introduces a new function cs_etm__end_block(), at the end of
trace block this function is invoked to only flush branch stack entries
and thus can avoid to generate branch sample for stale packet.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 789707bc4e9e..ffc4fe5c0b7e 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1055,6 +1055,39 @@ swap_packet:
return err;
}

+static int cs_etm__end_block(struct cs_etm_queue *etmq)
+{
+ int err;
+
+ /*
+ * It has no new packet coming and 'etmq->packet' contains the stale
+ * packet which was set at the previous time with packets swapping;
+ * so skip to generate branch sample to avoid stale packet.
+ *
+ * For this case only flush branch stack and generate a last branch
+ * event for the branches left in the circular buffer at the end of
+ * the trace.
+ */
+ if (etmq->etm->synth_opts.last_branch &&
+ etmq->prev_packet->sample_type == CS_ETM_RANGE) {
+ /*
+ * Use the address of the end of the last reported execution
+ * range.
+ */
+ u64 addr = cs_etm__last_executed_instr(etmq->prev_packet);
+
+ err = cs_etm__synth_instruction_sample(
+ etmq, addr,
+ etmq->period_instructions);
+ if (err)
+ return err;
+
+ etmq->period_instructions = 0;
+ }
+
+ return 0;
+}
+
static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
{
struct cs_etm_auxtrace *etm = etmq->etm;
@@ -1137,7 +1170,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)

if (err == 0)
/* Flush any remaining branch stack entries */
- err = cs_etm__flush(etmq);
+ err = cs_etm__end_block(etmq);
}

return err;

Subject: [tip:perf/core] perf cs-etm: Refactor enumeration cs_etm_sample_type

Commit-ID: cfc1d4276b0453a752d8b31daf34f7f2dbd55e82
Gitweb: https://git.kernel.org/tip/cfc1d4276b0453a752d8b31daf34f7f2dbd55e82
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:24 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Refactor enumeration cs_etm_sample_type

The values in enumeration cs_etm_sample_type are defined with setting
bit N for each packet type, this is not suggested in the usual case.

This patch refactor cs_etm_sample_type by converting from bit shifting
values to continuous numbers.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index b295dd2b8292..3819a04a2c27 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -23,9 +23,9 @@ struct cs_etm_buffer {
};

enum cs_etm_sample_type {
- CS_ETM_EMPTY = 0,
- CS_ETM_RANGE = 1 << 0,
- CS_ETM_TRACE_ON = 1 << 1,
+ CS_ETM_EMPTY,
+ CS_ETM_RANGE,
+ CS_ETM_TRACE_ON,
};

enum cs_etm_isa {

Subject: [tip:perf/core] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Commit-ID: 37bb37168dc1b1f5e3ac791aeecd14ef980764fd
Gitweb: https://git.kernel.org/tip/37bb37168dc1b1f5e3ac791aeecd14ef980764fd
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:26 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Treat NO_SYNC element as trace discontinuity

The CoreSight tracer driver might insert barrier packets between
different buffers, thus the decoder can spot the boundaries based on the
barrier packet; it is possible for the decoder to hit a barrier packet
and emit a NO_SYNC element, then the decoder will find a periodic
synchronisation point inside that next trace block that starts the trace
again but does not have the TRACE_ON element as indicator - usually
because this trace block has wrapped the buffer so we have lost the
original point when the trace was enabled.

In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC
in the middle of the tracing stream, but as we were not handling the
NO_SYNC element properly this ends up making users miss the
discontinuity indications.

Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
output from the decoder, both indicate that the trace data is
discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace
discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so
cs-etm can handle the discontinuity for this case, finally it saves the
last trace data for the previous trace block and restart samples for the
new block.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: coresight ml <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 1039f364f4cc..bee026e76a4c 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -410,7 +410,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
case OCSD_GEN_TRC_ELEM_NO_SYNC:
- break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
trace_chan_id);

Subject: [tip:perf/core] perf cs-etm: Rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY

Commit-ID: 49ccf87bfb9e8517ab2f019e95845760c4c3a562
Gitweb: https://git.kernel.org/tip/49ccf87bfb9e8517ab2f019e95845760c4c3a562
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:25 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Rename CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY

TRACE_ON element is used at the beginning of trace, it also can be
appeared in the middle of trace data to indicate discontinuity; for
example, it's possible to see multiple TRACE_ON elements in the trace
stream if the trace is being limited by address range filtering.

Furthermore, except TRACE_ON element is for discontinuity, NO_SYNC and
EO_TRACE also can be used to indicate discontinuity, though they are
used for different scenarios for which the trace is interrupted.

This patch renames sample type CS_ETM_TRACE_ON to CS_ETM_DISCONTINUITY,
firstly the new name describes more closely the purpose of the packet;
secondly this is a preparation for other output elements which also
cause the trace discontinuity thus they can share the same one packet
type.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 +++++-----
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 2 +-
tools/perf/util/cs-etm.c | 12 ++++++------
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 97b39b1f5759..1039f364f4cc 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -390,11 +390,11 @@ cs_etm_decoder__buffer_range(struct cs_etm_decoder *decoder,
}

static ocsd_datapath_resp_t
-cs_etm_decoder__buffer_trace_on(struct cs_etm_decoder *decoder,
- const uint8_t trace_chan_id)
+cs_etm_decoder__buffer_discontinuity(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
{
return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
- CS_ETM_TRACE_ON);
+ CS_ETM_DISCONTINUITY);
}

static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
@@ -412,8 +412,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
case OCSD_GEN_TRC_ELEM_NO_SYNC:
break;
case OCSD_GEN_TRC_ELEM_TRACE_ON:
- resp = cs_etm_decoder__buffer_trace_on(decoder,
- trace_chan_id);
+ resp = cs_etm_decoder__buffer_discontinuity(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
resp = cs_etm_decoder__buffer_range(decoder, elem,
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index 3819a04a2c27..a27231722e27 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -25,7 +25,7 @@ struct cs_etm_buffer {
enum cs_etm_sample_type {
CS_ETM_EMPTY,
CS_ETM_RANGE,
- CS_ETM_TRACE_ON,
+ CS_ETM_DISCONTINUITY,
};

enum cs_etm_isa {
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ffc4fe5c0b7e..cea3158915d3 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -562,8 +562,8 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,

static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
{
- /* Returns 0 for the CS_ETM_TRACE_ON packet */
- if (packet->sample_type == CS_ETM_TRACE_ON)
+ /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
+ if (packet->sample_type == CS_ETM_DISCONTINUITY)
return 0;

return packet->start_addr;
@@ -572,8 +572,8 @@ static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
static inline
u64 cs_etm__last_executed_instr(const struct cs_etm_packet *packet)
{
- /* Returns 0 for the CS_ETM_TRACE_ON packet */
- if (packet->sample_type == CS_ETM_TRACE_ON)
+ /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
+ if (packet->sample_type == CS_ETM_DISCONTINUITY)
return 0;

return packet->end_addr - packet->last_instr_size;
@@ -972,7 +972,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
bool generate_sample = false;

/* Generate sample for tracing on packet */
- if (etmq->prev_packet->sample_type == CS_ETM_TRACE_ON)
+ if (etmq->prev_packet->sample_type == CS_ETM_DISCONTINUITY)
generate_sample = true;

/* Generate sample for branch taken packet */
@@ -1148,7 +1148,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
*/
cs_etm__sample(etmq);
break;
- case CS_ETM_TRACE_ON:
+ case CS_ETM_DISCONTINUITY:
/*
* Discontinuity in trace, flush
* previous branch stack

Subject: [tip:perf/core] perf cs-etm: Generate branch sample for exception packet

Commit-ID: 7100b12cf474f575658e6e4a834f0e8a0b3c9867
Gitweb: https://git.kernel.org/tip/7100b12cf474f575658e6e4a834f0e8a0b3c9867
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:28 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:24:00 -0300

perf cs-etm: Generate branch sample for exception packet

The exception packet appears as one element with 'elem_type' ==
OCSD_GEN_TRC_ELEM_EXCEPTION or OCSD_GEN_TRC_ELEM_EXCEPTION_RET, which is
present for exception entry and exit respectively. The decoder sets the
packet fields 'packet->exc' and 'packet->exc_ret' to indicate the
exception packets; but exception packets don't have a dedicated sample
type and shares the same sample type CS_ETM_RANGE with normal
instruction packets.

As a result, the exception packets are taken as normal instruction
packets and this introduces confusion in mixing different packet types.
Furthermore, these instruction range packets will be processed for
branch samples only when 'packet->last_instr_taken_branch' is true,
otherwise they will be omitted, this can introduce a mess for exception
and exception returning due to not having the complete address range
info for context switching.

To process exception packets properly, this patch introduces two new
sample types: CS_ETM_EXCEPTION and CS_ETM_EXCEPTION_RET; these two types
of packets will be handled by cs_etm__exception(). The function
cs_etm__exception() forces setting the previous CS_ETM_RANGE packet flag
'prev_packet->last_instr_taken_branch' to true, this matches well with
the program flow when the exception is trapped from user space to kernel
space, no matter if the most recent flow has branch taken or not; this
is also safe for returning to user space after exception handling.

After exception packets have their own sample type, the packet fields
'packet->exc' and 'packet->exc_ret' aren't needed anymore, so remove
them.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: coresight ml <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 26 +++++++++++++++++------
tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 4 ++--
tools/perf/util/cs-etm.c | 28 +++++++++++++++++++++++++
3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index cda6f074bd03..8c155575c6c5 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -290,8 +290,6 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder)
decoder->packet_buffer[i].instr_count = 0;
decoder->packet_buffer[i].last_instr_taken_branch = false;
decoder->packet_buffer[i].last_instr_size = 0;
- decoder->packet_buffer[i].exc = false;
- decoder->packet_buffer[i].exc_ret = false;
decoder->packet_buffer[i].cpu = INT_MIN;
}
}
@@ -319,8 +317,6 @@ cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder,

decoder->packet_buffer[et].sample_type = sample_type;
decoder->packet_buffer[et].isa = CS_ETM_ISA_UNKNOWN;
- decoder->packet_buffer[et].exc = false;
- decoder->packet_buffer[et].exc_ret = false;
decoder->packet_buffer[et].cpu = *((int *)inode->priv);
decoder->packet_buffer[et].start_addr = CS_ETM_INVAL_ADDR;
decoder->packet_buffer[et].end_addr = CS_ETM_INVAL_ADDR;
@@ -397,6 +393,22 @@ cs_etm_decoder__buffer_discontinuity(struct cs_etm_decoder *decoder,
CS_ETM_DISCONTINUITY);
}

+static ocsd_datapath_resp_t
+cs_etm_decoder__buffer_exception(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
+{
+ return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
+ CS_ETM_EXCEPTION);
+}
+
+static ocsd_datapath_resp_t
+cs_etm_decoder__buffer_exception_ret(struct cs_etm_decoder *decoder,
+ const uint8_t trace_chan_id)
+{
+ return cs_etm_decoder__buffer_packet(decoder, trace_chan_id,
+ CS_ETM_EXCEPTION_RET);
+}
+
static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
const void *context,
const ocsd_trc_index_t indx __maybe_unused,
@@ -420,10 +432,12 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_EXCEPTION:
- decoder->packet_buffer[decoder->tail].exc = true;
+ resp = cs_etm_decoder__buffer_exception(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_EXCEPTION_RET:
- decoder->packet_buffer[decoder->tail].exc_ret = true;
+ resp = cs_etm_decoder__buffer_exception_ret(decoder,
+ trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
case OCSD_GEN_TRC_ELEM_ADDR_NACC:
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index a27231722e27..a6407d41598f 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -26,6 +26,8 @@ enum cs_etm_sample_type {
CS_ETM_EMPTY,
CS_ETM_RANGE,
CS_ETM_DISCONTINUITY,
+ CS_ETM_EXCEPTION,
+ CS_ETM_EXCEPTION_RET,
};

enum cs_etm_isa {
@@ -43,8 +45,6 @@ struct cs_etm_packet {
u32 instr_count;
u8 last_instr_taken_branch;
u8 last_instr_size;
- u8 exc;
- u8 exc_ret;
int cpu;
};

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index cea3158915d3..27a374ddf661 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1000,6 +1000,25 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
return 0;
}

+static int cs_etm__exception(struct cs_etm_queue *etmq)
+{
+ /*
+ * When the exception packet is inserted, whether the last instruction
+ * in previous range packet is taken branch or not, we need to force
+ * to set 'prev_packet->last_instr_taken_branch' to true. This ensures
+ * to generate branch sample for the instruction range before the
+ * exception is trapped to kernel or before the exception returning.
+ *
+ * The exception packet includes the dummy address values, so don't
+ * swap PACKET with PREV_PACKET. This keeps PREV_PACKET to be useful
+ * for generating instruction and branch samples.
+ */
+ if (etmq->prev_packet->sample_type == CS_ETM_RANGE)
+ etmq->prev_packet->last_instr_taken_branch = true;
+
+ return 0;
+}
+
static int cs_etm__flush(struct cs_etm_queue *etmq)
{
int err = 0;
@@ -1148,6 +1167,15 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
*/
cs_etm__sample(etmq);
break;
+ case CS_ETM_EXCEPTION:
+ case CS_ETM_EXCEPTION_RET:
+ /*
+ * If the exception packet is coming,
+ * make sure the previous instruction
+ * range packet to be handled properly.
+ */
+ cs_etm__exception(etmq);
+ break;
case CS_ETM_DISCONTINUITY:
/*
* Discontinuity in trace, flush

Subject: [tip:perf/core] perf cs-etm: Treat EO_TRACE element as trace discontinuity

Commit-ID: 02e7e2509e939b50f4f1b14c08b97f60256df3ea
Gitweb: https://git.kernel.org/tip/02e7e2509e939b50f4f1b14c08b97f60256df3ea
Author: Leo Yan <[email protected]>
AuthorDate: Tue, 11 Dec 2018 15:38:27 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:23:59 -0300

perf cs-etm: Treat EO_TRACE element as trace discontinuity

If the decoder outputs an EO_TRACE element, it means the end of the
trace buffer; this is a discontinuity and in this case the end of trace
data needs to be saved.

This patch generates a CS_ETM_DISCONTINUITY packet for the EO_TRACE
element hereby flushing the end of trace data in cs-etm.c.

Signed-off-by: Leo Yan <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Robert Walker <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index bee026e76a4c..cda6f074bd03 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -409,6 +409,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
switch (elem->elem_type) {
case OCSD_GEN_TRC_ELEM_UNKNOWN:
break;
+ case OCSD_GEN_TRC_ELEM_EO_TRACE:
case OCSD_GEN_TRC_ELEM_NO_SYNC:
case OCSD_GEN_TRC_ELEM_TRACE_ON:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
@@ -425,7 +426,6 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
decoder->packet_buffer[decoder->tail].exc_ret = true;
break;
case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
- case OCSD_GEN_TRC_ELEM_EO_TRACE:
case OCSD_GEN_TRC_ELEM_ADDR_NACC:
case OCSD_GEN_TRC_ELEM_TIMESTAMP:
case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: