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 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 0004 is used to support NO_SYNC packet, otherwise the trace decoding cannot reflect the tracing discontinuity caused by NO_SYNC packet.
Patch 0005 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 0006 is used to generate branch sample for exception packets.
This patch series is applied on the acme's perf/core branch [1] with latest commit 4085fed6373f ("perf trace: Add ordered processing"); Since Rob's patch 'perf: Support for Arm A32/T32 instruction sets in CoreSight trace' has been merged into perf/core branch, this patch series can directly be applied onto perf/core branch.
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 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 (6): perf cs-etm: Correct packets swapping in cs_etm__flush() perf cs-etm: Avoid stale branch samples when flush packet 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 | 40 +++++++++---- tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 10 ++-- tools/perf/util/cs-etm.c | 77 ++++++++++++++++++++++--- 3 files changed, 102 insertions(+), 25 deletions(-)
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().
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org --- 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.
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com 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 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;
On Mon, Dec 10, 2018 at 04:52:57PM +0800, Leo Yan wrote:
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com 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 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;
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
-- 2.7.4
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.
This patch also refactors the enumerations in cs_etm_sample_type by converting from bit shifting value to continuous numbers.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 +++++----- tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 6 +++--- tools/perf/util/cs-etm.c | 12 ++++++------ 3 files changed, 14 insertions(+), 14 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..a3994f1 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -391,11 +391,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( @@ -414,8 +414,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( decoder->trace_on = false; 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); decoder->trace_on = true; break; case OCSD_GEN_TRC_ELEM_INSTR_RANGE: 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..a272317 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_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
Hi Leo,
On Mon, Dec 10, 2018 at 04:52:58PM +0800, Leo Yan wrote:
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.
This patch also refactors the enumerations in cs_etm_sample_type by converting from bit shifting value to continuous numbers.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 +++++----- tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 6 +++--- tools/perf/util/cs-etm.c | 12 ++++++------ 3 files changed, 14 insertions(+), 14 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..a3994f1 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -391,11 +391,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( @@ -414,8 +414,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( decoder->trace_on = false; 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,
decoder->trace_on = true; break; case OCSD_GEN_TRC_ELEM_INSTR_RANGE:trace_chan_id);
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..a272317 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_DISCONTINUITY,
};
I'm in agreement with what you're doing in this patch but the above is a different change and you can't pack it in here. Just split it off in a separate patch and everything will be fine.
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
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.
Credit to Mike Leach and Robert Walker who made me clear for underlying mechanism for NO_SYNC element, Mike also shared with me for detailed explanation for why we can treat NO_SYNC and TRACE_ON elements as the same, so this commit log reused most of his explanation.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index a3994f1..46b67f1 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -411,6 +411,8 @@ 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: + resp = cs_etm_decoder__buffer_discontinuity(decoder, + trace_chan_id); decoder->trace_on = false; break; case OCSD_GEN_TRC_ELEM_TRACE_ON:
On Mon, Dec 10, 2018 at 04:52:59PM +0800, Leo Yan wrote:
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.
Credit to Mike Leach and Robert Walker who made me clear for underlying mechanism for NO_SYNC element, Mike also shared with me for detailed explanation for why we can treat NO_SYNC and TRACE_ON elements as the same, so this commit log reused most of his explanation.
Credit and thank you note such as this one go in the cover letter. With this change:
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index a3994f1..46b67f1 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -411,6 +411,8 @@ 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:
resp = cs_etm_decoder__buffer_discontinuity(decoder,
decoder->trace_on = false; break; case OCSD_GEN_TRC_ELEM_TRACE_ON:trace_chan_id);
-- 2.7.4
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org --- 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 46b67f1..bcb5c98 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -411,6 +411,7 @@ 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: + case OCSD_GEN_TRC_ELEM_EO_TRACE: resp = cs_etm_decoder__buffer_discontinuity(decoder, trace_chan_id); decoder->trace_on = false; @@ -431,7 +432,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:
On Mon, Dec 10, 2018 at 04:53:00PM +0800, Leo Yan wrote:
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org
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 46b67f1..bcb5c98 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -411,6 +411,7 @@ 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:
- case OCSD_GEN_TRC_ELEM_EO_TRACE: resp = cs_etm_decoder__buffer_discontinuity(decoder, trace_chan_id);
If you were to get rid of decoder::trace_on at the beginning of this set you could put NO_SYNC, EO_TRACE and TRACE_ON together and call cs_etm_decoder__buffer_discontinuity() only once. I wouldn't mention it if you did not have to respin but since you do, might as well just do it. But that's entirely up to you considering, at least in my opinion, that you have addressed all of Mike and Rob' comments.
If you do not want to deal with decoder::trace_on as part of this set:
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
decoder->trace_on = false;
@@ -431,7 +432,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
Hi Mathieu,
On Mon, Dec 10, 2018 at 04:04:45PM -0700, Mathieu Poirier wrote:
On Mon, Dec 10, 2018 at 04:53:00PM +0800, Leo Yan wrote:
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org
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 46b67f1..bcb5c98 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -411,6 +411,7 @@ 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:
- case OCSD_GEN_TRC_ELEM_EO_TRACE: resp = cs_etm_decoder__buffer_discontinuity(decoder, trace_chan_id);
If you were to get rid of decoder::trace_on at the beginning of this set you could put NO_SYNC, EO_TRACE and TRACE_ON together and call cs_etm_decoder__buffer_discontinuity() only once. I wouldn't mention it if you did not have to respin but since you do, might as well just do it. But that's entirely up to you considering, at least in my opinion, that you have addressed all of Mike and Rob' comments.
Thanks for suggestion, this makes sense. Will spin new patch series to address this and also follow up other suggestions for this series.
Thanks, Leo Yan
If you do not want to deal with decoder::trace_on as part of this set:
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
decoder->trace_on = false;
@@ -431,7 +432,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
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org --- 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 bcb5c98..6d89d0e 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -291,8 +291,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; } } @@ -320,8 +318,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; @@ -398,6 +394,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, @@ -426,10 +438,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
On Mon, Dec 10, 2018 at 04:53:01PM +0800, Leo Yan wrote:
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.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org
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 bcb5c98..6d89d0e 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -291,8 +291,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].cpu = INT_MIN; }decoder->packet_buffer[i].exc_ret = false;
} @@ -320,8 +318,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;
@@ -398,6 +394,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, @@ -426,10 +438,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,
break; case OCSD_GEN_TRC_ELEM_EXCEPTION_RET:trace_chan_id);
decoder->packet_buffer[decoder->tail].exc_ret = true;
resp = cs_etm_decoder__buffer_exception_ret(decoder,
break; case OCSD_GEN_TRC_ELEM_PE_CONTEXT: case OCSD_GEN_TRC_ELEM_ADDR_NACC:trace_chan_id);
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
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org