| Message ID | 1518011845-24063-3-git-send-email-robert.walker@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 07, 2018 at 01:57:25PM +0000, Robert Walker wrote: > There may be discontinuities in the ETM trace stream due to overflows or > ETM configuration for selective trace. This patch emits an instruction > sample with the pending branch stack when a TRACE ON packet occurs > indicating a discontinuity in the trace data. > > A new packet type CS_ETM_TRACE_ON is added, which is emitted by the low > level decoder when a TRACE ON occurs. The higher level decoder flushes the > branch stack when this packet is emitted. > > Signed-off-by: Robert Walker <robert.walker@arm.com> > --- > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 9 +++ > tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 1 + > tools/perf/util/cs-etm.c | 84 +++++++++++++++++-------- > 3 files changed, 69 insertions(+), 25 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 8ff69df..640af88 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -328,7 +328,14 @@ cs_etm_decoder__buffer_range(struct cs_etm_decoder *decoder, > } > > return ret; > +} > > +static ocsd_datapath_resp_t > +cs_etm_decoder__buffer_trace_on(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); > } > > static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > @@ -347,6 +354,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); > 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 a4fdd28..743f5f4 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > @@ -24,6 +24,7 @@ struct cs_etm_buffer { > > enum cs_etm_sample_type { > CS_ETM_RANGE = 1 << 0, > + CS_ETM_TRACE_ON = 1 << 1, > }; > > struct cs_etm_packet { > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 6777246..a8d07bd 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -866,6 +866,8 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) > * PREV_PACKET is a branch. > */ > if (etm->synth_opts.last_branch && > + etmq->prev_packet && > + etmq->prev_packet->sample_type == CS_ETM_RANGE && > etmq->prev_packet->last_instr_taken_branch) > cs_etm__update_last_branch_rb(etmq); > > @@ -918,6 +920,40 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) > return 0; > } > > +static int cs_etm__flush(struct cs_etm_queue *etmq) > +{ > + int err = 0; > + struct cs_etm_packet *tmp; > + > + if (etmq->etm->synth_opts.last_branch && > + etmq->prev_packet && > + etmq->prev_packet->sample_type == CS_ETM_RANGE) { > + /* > + * Generate a last branch event for the branches left in the > + * circular buffer at the end of the trace. > + * > + * 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); > + etmq->period_instructions = 0; > + > + /* > + * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for > + * the next incoming packet. > + */ > + tmp = etmq->packet; > + etmq->packet = etmq->prev_packet; > + etmq->prev_packet = tmp; > + } > + > + return err; > +} > + > static int cs_etm__run_decoder(struct cs_etm_queue *etmq) > { > struct cs_etm_auxtrace *etm = etmq->etm; > @@ -946,20 +982,19 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) > /* Run trace decoder until buffer consumed or end of trace */ > do { > processed = 0; > - this... > err = cs_etm_decoder__process_data_block( > etmq->decoder, > etmq->offset, > &buffer.buf[buffer_used], > buffer.len - buffer_used, > &processed); > - and this should have gone in the first patch. > if (err) > return err; > > etmq->offset += processed; > buffer_used += processed; > > + /* Process each packet in this chunk */ And probably this too. With the above changes: Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org> > while (1) { > err = cs_etm_decoder__get_packet(etmq->decoder, > etmq->packet); > @@ -970,32 +1005,31 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) > */ > break; > > - /* > - * If the packet contains an instruction > - * range, generate instruction sequence > - * events. > - */ > - if (etmq->packet->sample_type & CS_ETM_RANGE) > - err = cs_etm__sample(etmq); > + switch (etmq->packet->sample_type) { > + case CS_ETM_RANGE: > + /* > + * If the packet contains an instruction > + * range, generate instruction sequence > + * events. > + */ > + cs_etm__sample(etmq); > + break; > + case CS_ETM_TRACE_ON: > + /* > + * Discontinuity in trace, flush > + * previous branch stack > + */ > + cs_etm__flush(etmq); > + break; > + default: > + break; > + } > } > } while (buffer.len > buffer_used); > > - /* > - * Generate a last branch event for the branches left in > - * the circular buffer at the end of the trace. > - */ > - if (etm->sample_instructions && > - etmq->etm->synth_opts.last_branch) { > - struct branch_stack *bs = etmq->last_branch_rb; > - struct branch_entry *be = > - &bs->entries[etmq->last_branch_pos]; > - > - err = cs_etm__synth_instruction_sample( > - etmq, be->to, etmq->period_instructions); > - if (err) > - return err; > - } > - > + if (err == 0) > + /* Flush any remaining branch stack entries */ > + err = cs_etm__flush(etmq); > } > > return err; > -- > 2.7.4 >
Em Tue, Feb 13, 2018 at 03:22:33PM -0700, Mathieu Poirier escreveu: > On Wed, Feb 07, 2018 at 01:57:25PM +0000, Robert Walker wrote: > > @@ -946,20 +982,19 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) > > /* Run trace decoder until buffer consumed or end of trace */ > > do { > > processed = 0; > > - > > this... > > > err = cs_etm_decoder__process_data_block( > > etmq->decoder, > > etmq->offset, > > &buffer.buf[buffer_used], > > buffer.len - buffer_used, > > &processed); > > - > > and this should have gone in the first patch. > > > if (err) > > return err; > > > > etmq->offset += processed; > > buffer_used += processed; > > > > + /* Process each packet in this chunk */ > > And probably this too. > > With the above changes: > > Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org> Hi Robert, Can you please address Mathieu's comments, and if you agree with them, resubmit? Thanks! - Arnaldo
> -----Original Message----- > From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] > Sent: 15 February 2018 14:57 > To: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Robert Walker <Robert.Walker@arm.com>; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > coresight@lists.linaro.org > Subject: Re: [PATCH 2/2] perf inject: Emit instruction records on ETM trace > discontinuity > > Em Tue, Feb 13, 2018 at 03:22:33PM -0700, Mathieu Poirier escreveu: > > On Wed, Feb 07, 2018 at 01:57:25PM +0000, Robert Walker wrote: > > > @@ -946,20 +982,19 @@ static int cs_etm__run_decoder(struct > cs_etm_queue *etmq) > > > /* Run trace decoder until buffer consumed or end of trace > */ > > > do { > > > processed = 0; > > > - > > > > this... > > > > > err = cs_etm_decoder__process_data_block( > > > etmq->decoder, > > > etmq->offset, > > > &buffer.buf[buffer_used], > > > buffer.len - buffer_used, > > > &processed); > > > - > > > > and this should have gone in the first patch. > > > > > if (err) > > > return err; > > > > > > etmq->offset += processed; > > > buffer_used += processed; > > > > > > + /* Process each packet in this chunk */ > > > > And probably this too. > > > > With the above changes: > > > > Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Hi Robert, > > Can you please address Mathieu's comments, and if you agree with > them, resubmit? > > Thanks! > > - Arnaldo Hi Arnaldo, I've addressed Mathieu's comments and resubmitted them yesterday (https://lkml.org/lkml/2018/2/14/185). Regards Rob
Em Thu, Feb 15, 2018 at 03:22:52PM -0000, Robert Walker escreveu: > > From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] > > Can you please address Mathieu's comments, and if you agree with > > them, resubmit? > Hi Arnaldo, > I've addressed Mathieu's comments and resubmitted them yesterday > (https://lkml.org/lkml/2018/2/14/185). Thanks, I'm processing them now, - Arnaldo
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 8ff69df..640af88 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -328,7 +328,14 @@ cs_etm_decoder__buffer_range(struct cs_etm_decoder *decoder, } return ret; +} +static ocsd_datapath_resp_t +cs_etm_decoder__buffer_trace_on(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); } static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( @@ -347,6 +354,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); 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 a4fdd28..743f5f4 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h @@ -24,6 +24,7 @@ struct cs_etm_buffer { enum cs_etm_sample_type { CS_ETM_RANGE = 1 << 0, + CS_ETM_TRACE_ON = 1 << 1, }; struct cs_etm_packet { diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 6777246..a8d07bd 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -866,6 +866,8 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) * PREV_PACKET is a branch. */ if (etm->synth_opts.last_branch && + etmq->prev_packet && + etmq->prev_packet->sample_type == CS_ETM_RANGE && etmq->prev_packet->last_instr_taken_branch) cs_etm__update_last_branch_rb(etmq); @@ -918,6 +920,40 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) return 0; } +static int cs_etm__flush(struct cs_etm_queue *etmq) +{ + int err = 0; + struct cs_etm_packet *tmp; + + if (etmq->etm->synth_opts.last_branch && + etmq->prev_packet && + etmq->prev_packet->sample_type == CS_ETM_RANGE) { + /* + * Generate a last branch event for the branches left in the + * circular buffer at the end of the trace. + * + * 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); + etmq->period_instructions = 0; + + /* + * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for + * the next incoming packet. + */ + tmp = etmq->packet; + etmq->packet = etmq->prev_packet; + etmq->prev_packet = tmp; + } + + return err; +} + static int cs_etm__run_decoder(struct cs_etm_queue *etmq) { struct cs_etm_auxtrace *etm = etmq->etm; @@ -946,20 +982,19 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) /* Run trace decoder until buffer consumed or end of trace */ do { processed = 0; - err = cs_etm_decoder__process_data_block( etmq->decoder, etmq->offset, &buffer.buf[buffer_used], buffer.len - buffer_used, &processed); - if (err) return err; etmq->offset += processed; buffer_used += processed; + /* Process each packet in this chunk */ while (1) { err = cs_etm_decoder__get_packet(etmq->decoder, etmq->packet); @@ -970,32 +1005,31 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) */ break; - /* - * If the packet contains an instruction - * range, generate instruction sequence - * events. - */ - if (etmq->packet->sample_type & CS_ETM_RANGE) - err = cs_etm__sample(etmq); + switch (etmq->packet->sample_type) { + case CS_ETM_RANGE: + /* + * If the packet contains an instruction + * range, generate instruction sequence + * events. + */ + cs_etm__sample(etmq); + break; + case CS_ETM_TRACE_ON: + /* + * Discontinuity in trace, flush + * previous branch stack + */ + cs_etm__flush(etmq); + break; + default: + break; + } } } while (buffer.len > buffer_used); - /* - * Generate a last branch event for the branches left in - * the circular buffer at the end of the trace. - */ - if (etm->sample_instructions && - etmq->etm->synth_opts.last_branch) { - struct branch_stack *bs = etmq->last_branch_rb; - struct branch_entry *be = - &bs->entries[etmq->last_branch_pos]; - - err = cs_etm__synth_instruction_sample( - etmq, be->to, etmq->period_instructions); - if (err) - return err; - } - + if (err == 0) + /* Flush any remaining branch stack entries */ + err = cs_etm__flush(etmq); } return err;
There may be discontinuities in the ETM trace stream due to overflows or ETM configuration for selective trace. This patch emits an instruction sample with the pending branch stack when a TRACE ON packet occurs indicating a discontinuity in the trace data. A new packet type CS_ETM_TRACE_ON is added, which is emitted by the low level decoder when a TRACE ON occurs. The higher level decoder flushes the branch stack when this packet is emitted. Signed-off-by: Robert Walker <robert.walker@arm.com> --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 9 +++ tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 1 + tools/perf/util/cs-etm.c | 84 +++++++++++++++++-------- 3 files changed, 69 insertions(+), 25 deletions(-)