[PATCH v2 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[PATCH v2 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[Leo Yan]

Due the current code is missing to handle cs-etm start tracing packet
and CS_ETM_TRACE_ON packet, we fail to generate branch sample for them.

This patch series is to fix cs-etm tracing packet handling:

Patch 0001 is to add invalid address macro for readable coding;

Patch 0002 is one minor fixing to return error code for instruction
sample failure;

Patches 0003~0006 are fixing patches for start tracing packet
and CS_ETM_TRACE_ON packet.

This patch series is applied to Linus tree on 4.18-rc4 with latest
commit: 1e09177acae3 ("Merge tag 'mips_fixes_4.18_3' of
git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux").

This patch series has been verified on Hikey620 platform with below two
commands:
perf script --itrace=i10000il128 -F cpu,event,ip,addr,sym -k ./vmlinux
perf script -F cpu,event,ip,addr,sym -k ./vmlinux

[1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git

Changes from v1:
* Rebased on Linus mainline kernel 4.18-rc4;
* Added Mathieu 'reviewed' tags in patches.


Leo Yan (6):
  perf cs-etm: Introduce invalid address macro
  perf cs-etm: Bail out immediately for instruction sample failure
  perf cs-etm: Fix start tracing packet handling
  perf cs-etm: Support dummy address value for CS_ETM_TRACE_ON packet
  perf cs-etm: Generate branch sample when receiving a CS_ETM_TRACE_ON
    packet
  perf cs-etm: Generate branch sample for CS_ETM_TRACE_ON packet

 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 ++--
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |  1 +
 tools/perf/util/cs-etm.c                        | 71 +++++++++++++++++++++----
 3 files changed, 68 insertions(+), 14 deletions(-)

-- 
2.7.4

[PATCH v2 1/6] perf cs-etm: Introduce invalid address macro

[Leo Yan]

This patch introduces invalid address macro and uses it to replace dummy
value '0xdeadbeefdeadbeefUL'.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 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 4d5fc37..938def6 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -31,6 +31,8 @@
 #endif
 #endif
 
+#define CS_ETM_INVAL_ADDR	0xdeadbeefdeadbeefUL
+
 struct cs_etm_decoder {
 	void *data;
 	void (*packet_printer)(const char *msg);
@@ -261,8 +263,8 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder)
 	decoder->tail = 0;
 	decoder->packet_count = 0;
 	for (i = 0; i < MAX_BUFFER; i++) {
-		decoder->packet_buffer[i].start_addr = 0xdeadbeefdeadbeefUL;
-		decoder->packet_buffer[i].end_addr = 0xdeadbeefdeadbeefUL;
+		decoder->packet_buffer[i].start_addr = CS_ETM_INVAL_ADDR;
+		decoder->packet_buffer[i].end_addr = CS_ETM_INVAL_ADDR;
 		decoder->packet_buffer[i].last_instr_taken_branch = false;
 		decoder->packet_buffer[i].exc = false;
 		decoder->packet_buffer[i].exc_ret = false;
@@ -295,8 +297,8 @@ cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder,
 	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 = 0xdeadbeefdeadbeefUL;
-	decoder->packet_buffer[et].end_addr = 0xdeadbeefdeadbeefUL;
+	decoder->packet_buffer[et].start_addr = CS_ETM_INVAL_ADDR;
+	decoder->packet_buffer[et].end_addr = CS_ETM_INVAL_ADDR;
 
 	if (decoder->packet_count == MAX_BUFFER - 1)
 		return OCSD_RESP_WAIT;
-- 
2.7.4

[PATCH v2 2/6] perf cs-etm: Bail out immediately for instruction sample failure

[Leo Yan]

If the instruction sample failure has happened, it isn't necessary to
execute to the end of the function cs_etm__flush().  This commit is to
bail out immediately and return the error code.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 822ba91..8b2c099 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -939,6 +939,9 @@ 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;
+
 		etmq->period_instructions = 0;
 
 		/*
-- 
2.7.4

[PATCH v2 3/6] perf cs-etm: Fix start tracing packet handling

[Leo Yan]

Usually the start tracing packet is a CS_ETM_TRACE_ON packet, this
packet is passed to cs_etm__flush();  cs_etm__flush() will check the
condition 'prev_packet->sample_type == CS_ETM_RANGE' but 'prev_packet'
is allocated by zalloc() so 'prev_packet->sample_type' is zero in
initialization and this condition is false.  So cs_etm__flush() will
directly bail out without handling the start tracing packet.

This patch is to introduce a new sample type CS_ETM_EMPTY, which is used
to indicate the packet is an empty packet.  cs_etm__flush() will swap
packets when it finds the previous packet is empty, so this can record
the start tracing packet into 'etmq->prev_packet'.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |  1 +
 tools/perf/util/cs-etm.c                        | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

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 743f5f4..612b575 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
@@ -23,6 +23,7 @@ struct cs_etm_buffer {
 };
 
 enum cs_etm_sample_type {
+	CS_ETM_EMPTY = 0,
 	CS_ETM_RANGE = 1 << 0,
 	CS_ETM_TRACE_ON = 1 << 1,
 };
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 8b2c099..ae7c9c88 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -924,8 +924,14 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
 	int err = 0;
 	struct cs_etm_packet *tmp;
 
+	if (!etmq->prev_packet)
+		return 0;
+
+	/* Handle start tracing packet */
+	if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
+		goto swap_packet;
+
 	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
@@ -944,6 +950,10 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
 
 		etmq->period_instructions = 0;
 
+	}
+
+swap_packet:
+	if (etmq->etm->synth_opts.last_branch) {
 		/*
 		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
 		 * the next incoming packet.
@@ -1023,6 +1033,13 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
 					 */
 					cs_etm__flush(etmq);
 					break;
+				case CS_ETM_EMPTY:
+					/*
+					 * Should not receive empty packet,
+					 * report error.
+					 */
+					pr_err("CS ETM Trace: empty packet\n");
+					return -EINVAL;
 				default:
 					break;
 				}
-- 
2.7.4

[PATCH v2 4/6] perf cs-etm: Support dummy address value for CS_ETM_TRACE_ON packet

[Leo Yan]

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, but the dummy value is pointless for
branch sample when we use 'perf script' command to check program flow.

This patch is a preparation to support CS_ETM_TRACE_ON packet for
branch sample, it converts the dummy address value to zero for more
readable; this is accomplished by cs_etm__last_executed_instr() and
cs_etm__first_executed_instr().  The later one is a new function
introduced by this patch.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ae7c9c88..976db84 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;
-- 
2.7.4

[PATCH v2 5/6] perf cs-etm: Generate branch sample when receiving a CS_ETM_TRACE_ON packet

[Leo Yan]

If one CS_ETM_TRACE_ON packet is inserted, we miss to generate branch
sample for the previous CS_ETM_RANGE packet.

This patch is to generate branch sample when receiving a CS_ETM_TRACE_ON
packet, so this can save complete info for the previous CS_ETM_RANGE
packet just before CS_ETM_TRACE_ON packet.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 976db84..d3b7942 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -935,6 +935,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)
@@ -965,6 +966,13 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
 
 	}
 
+	if (etm->sample_branches &&
+	    etmq->prev_packet->sample_type == CS_ETM_RANGE) {
+		err = cs_etm__synth_branch_sample(etmq);
+		if (err)
+			return err;
+	}
+
 swap_packet:
 	if (etmq->etm->synth_opts.last_branch) {
 		/*
-- 
2.7.4

[PATCH v2 6/6] perf cs-etm: Generate branch sample for CS_ETM_TRACE_ON packet

[Leo Yan]

CS_ETM_TRACE_ON packet itself can give the info that there have a
discontinuity in the trace, this patch is to add branch sample for
CS_ETM_TRACE_ON packet if it is inserted in the middle of CS_ETM_RANGE
packets; as result we can have hint for the trace discontinuity.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/cs-etm.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index d3b7942..2ae6402 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -910,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 branch taken 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) {
-- 
2.7.4

[PATCH v2 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[leo.yan at linaro.org]

Hi Arnaldo,

On Wed, Jul 11, 2018 at 03:45:39PM +0800, Leo Yan wrote:

Just want to confirm, I saw the first two patches in this serise have
been merged into perf/core branch [1], but the last 4 patches are
missed.

Could I know if you have trouble when you apply them?  Or anything
need me to follow up?  Otherwise you might miss them :)

Thanks,
Leo Yan

[1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core

> Leo Yan (6):
>   perf cs-etm: Introduce invalid address macro
>   perf cs-etm: Bail out immediately for instruction sample failure
>   perf cs-etm: Fix start tracing packet handling
>   perf cs-etm: Support dummy address value for CS_ETM_TRACE_ON packet
>   perf cs-etm: Generate branch sample when receiving a CS_ETM_TRACE_ON
>     packet
>   perf cs-etm: Generate branch sample for CS_ETM_TRACE_ON packet
> 
>  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 ++--
>  tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |  1 +
>  tools/perf/util/cs-etm.c                        | 71 +++++++++++++++++++++----
>  3 files changed, 68 insertions(+), 14 deletions(-)
> 
> -- 
> 2.7.4
> 

[PATCH v2 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[Arnaldo Carvalho de Melo]

Em Mon, Jul 23, 2018 at 09:35:07AM +0800, leo.yan at linaro.org escreveu:
> Hi Arnaldo,
> 
> On Wed, Jul 11, 2018 at 03:45:39PM +0800, Leo Yan wrote:
> 
> Just want to confirm, I saw the first two patches in this serise have
> been merged into perf/core branch [1], but the last 4 patches are
> missed.
> 
> Could I know if you have trouble when you apply them?  Or anything
> need me to follow up?  Otherwise you might miss them :)

Just this reminder was enough, checked that it has Mathieu's reviewed-by
tags, applied, will go thru my cross build containers, hopefully all
will test build ok and then it'll go to Ingo's direction.

Thanks,

- Arnaldo

[PATCH v2 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[leo.yan at linaro.org]

On Tue, Jul 31, 2018 at 11:24:45AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 23, 2018 at 09:35:07AM +0800, leo.yan at linaro.org escreveu:
> > Hi Arnaldo,
> > 
> > On Wed, Jul 11, 2018 at 03:45:39PM +0800, Leo Yan wrote:
> > 
> > Just want to confirm, I saw the first two patches in this serise have
> > been merged into perf/core branch [1], but the last 4 patches are
> > missed.
> > 
> > Could I know if you have trouble when you apply them?  Or anything
> > need me to follow up?  Otherwise you might miss them :)
> 
> Just this reminder was enough, checked that it has Mathieu's reviewed-by
> tags, applied, will go thru my cross build containers, hopefully all
> will test build ok and then it'll go to Ingo's direction.

Cool!  Thanks a lot, Arnaldo.