Re: [RFT v4 2/6] perf cs-etm: Generate branch sample for CS_ETM_TRACE_ON packet

Hi Mathieu,

Oa Thu, May 31, 2018 at 11:07:11AM -0600, Mathieu Poirier wrote:

On Thu, May 31, 2018 at 01:22:06PM +0800, Leo Yan wrote:

...

If one CS_ETM_TRACE_ON packet is inserted, we miss to handle it for branch samples. CS_ETM_TRACE_ON packet itself can give the info that there have a discontinuity in the trace, furthermore we also miss to generate branch sample for packets before and after CS_ETM_TRACE_ON packet.

This patch is to add branch sample handling if CS_ETM_TRACE_ON packet is inserted in the middle of CS_ETM_RANGE packets:

If there has one CS_ETM_TRACE_ON packet is coming, we firstly generate branch sample in the function cs_etm__flush(), this can save complete info for the previous CS_ETM_RANGE packet just before CS_ETM_TRACE_ON packet. We also generate branch sample for the new CS_ETM_RANGE packet after CS_ETM_TRACE_ON packet, this have two purposes, the first one purpose is to save the info for the new CS_ETM_RANGE packet, the second purpose is to save CS_ETM_TRACE_ON packet info so we can have hint for a discontinuity in the trace.

For CS_ETM_TRACE_ON packet, its fields 'packet->start_addr' and 'packet->end_addr' equal to 0xdeadbeefdeadbeefUL which are emitted in the decoder layer as dummy value. This patch is to convert these values to zeros for more readable; this is accomplished by functions cs_etm__last_executed_instr() and cs_etm__first_executed_instr(). The later one is a new function introduced by this patch.

Reviewed-by: Robert Walker robert.walker@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org

---

tools/perf/util/cs-etm.c | 52 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 67564c1..8ec2f2c 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -494,6 +494,10 @@ static inline void cs_etm__reset_last_branch_rb(struct cs_etm_queue *etmq) static inline u64 cs_etm__last_executed_instr(struct cs_etm_packet *packet) {

• /* Returns 0 for the CS_ETM_TRACE_ON packet */
• if (packet->sample_type == CS_ETM_TRACE_ON)

• return 0;
  

• /*
  • The packet records the execution range with an exclusive end address

@@ -505,6 +509,15 @@ static inline u64 cs_etm__last_executed_instr(struct cs_etm_packet *packet) return packet->end_addr - A64_INSTR_SIZE; } +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)

• return 0;
  

• return packet->start_addr;

+}

static inline u64 cs_etm__instr_count(const struct cs_etm_packet *packet) { /* @@ -546,7 +559,7 @@ static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq) be = &bs->entries[etmq->last_branch_pos]; be->from = cs_etm__last_executed_instr(etmq->prev_packet);

• be->to = etmq->packet->start_addr;

• be->to = cs_etm__first_executed_instr(etmq->packet); /* No support for mispredict */ be->flags.mispred = 0; be->flags.predicted = 1;

@@ -701,7 +714,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq) sample.ip = cs_etm__last_executed_instr(etmq->prev_packet); sample.pid = etmq->pid; sample.tid = etmq->tid;

• sample.addr = etmq->packet->start_addr;

• sample.addr = cs_etm__first_executed_instr(etmq->packet); sample.id = etmq->etm->branches_id; sample.stream_id = etmq->etm->branches_id; sample.period = 1;

@@ -897,13 +910,23 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) etmq->period_instructions = instrs_over; }

• if (etm->sample_branches &&

•    etmq->prev_packet &&
  

•    etmq->prev_packet->sample_type == CS_ETM_RANGE &&
  

•    etmq->prev_packet->last_instr_taken_branch) {
  

• ret = cs_etm__synth_branch_sample(etmq);
  

• if (ret)
  

• 	return ret;
  

• if (etm->sample_branches && etmq->prev_packet) {

• bool generate_sample = false;
  

• /* Generate sample for tracing on packet */
  

• if (etmq->prev_packet->sample_type == CS_ETM_TRACE_ON)
  

• 	generate_sample = true;
  

• /* Generate sample for normal branch packet */
  

• if (etmq->prev_packet->sample_type == CS_ETM_RANGE &&
  

•     etmq->prev_packet->last_instr_taken_branch)
  

• 	generate_sample = true;
  

• if (generate_sample) {
  

• 	ret = cs_etm__synth_branch_sample(etmq);
  

• 	if (ret)
  

• 		return ret;
  

• }
  

  }

if (etm->sample_branches || etm->synth_opts.last_branch) { @@ -922,6 +945,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) static int cs_etm__flush(struct cs_etm_queue *etmq) { int err = 0;

• struct cs_etm_auxtrace *etm = etmq->etm; struct cs_etm_packet *tmp;

if (!etmq->prev_packet) @@ -948,12 +972,20 @@ static int cs_etm__flush(struct cs_etm_queue *etmq) err = cs_etm__synth_instruction_sample( etmq, addr, etmq->period_instructions);

• if (err)
  

• 	return err;
  

To me this is a bug fix and should be in a separate patch.

...

etmq->period_instructions = 0;

}

• if (etm->sample_branches) {

• err = cs_etm__synth_branch_sample(etmq);
  

• if (err)
  

• 	return err;
  

• }

So is this - we should generate a branch sample when receiving a TRACE_ON packet.

Have splitted into a dedicated patch for 'generate a branch sample when receiving a TRACE_ON packet.' For more clear logic in the patch series, I tried to split into 3 patches:

- Support dummy address in CS_ETM_TRACE_ON packet; - Fix patch for generating a branch sample when receiving a CS_ETM_TRACE_ON packet; - Generate branch sample for CS_ETM_TRACE_ON packet

Please let me know if it's doable or any better idea?

Thanks, Leo Yan

...

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