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

[PATCH v1 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 on acme tree [1] on branch perf/core with
latest commit: e238cf2e3d2e ("perf intel-pt: Fix packet decoding of CYC
packets").  Also applied successfully this patch series on Linus tree
on 4.18-rc1.

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


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 v1 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'.

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 v1 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.

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 v1 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'.

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 v1 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.

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 v1 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.

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 v1 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.

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 v1 0/6] perf cs-etm: Fix tracing packet handling and minor refactoring

[Mathieu Poirier]

On Sun, 17 Jun 2018 at 23:10, Leo Yan <leo.yan@linaro.org> wrote:
>
> 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 on acme tree [1] on branch perf/core with
> latest commit: e238cf2e3d2e ("perf intel-pt: Fix packet decoding of CYC
> packets").  Also applied successfully this patch series on Linus tree
> on 4.18-rc1.
>
> 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
>
>
> 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(-)

Good day Arnaldo,

I am good with this set:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Please consider for inclusion in your tree if you are satisfied with the work.

Regards,
Mathieu

>
> --
> 2.7.4
>

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

[Arnaldo Carvalho de Melo]

Em Tue, Jun 19, 2018 at 11:46:02AM -0600, Mathieu Poirier escreveu:
> On Sun, 17 Jun 2018 at 23:10, Leo Yan <leo.yan@linaro.org> wrote:
> >
> > 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 on acme tree [1] on branch perf/core with
> > latest commit: e238cf2e3d2e ("perf intel-pt: Fix packet decoding of CYC
> > packets").  Also applied successfully this patch series on Linus tree
> > on 4.18-rc1.
> >
> > 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
> >
> >
> > 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(-)
> 
> Good day Arnaldo,
> 
> I am good with this set:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Please consider for inclusion in your tree if you are satisfied with the work.

I'll take a look and get it into perf/core, now I'm concentrating on
perf/urgent work.

Thanks,

- Arnaldo

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

[leo.yan at linaro.org]

Hi Arnaldo,

On Tue, Jun 19, 2018 at 03:19:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jun 19, 2018 at 11:46:02AM -0600, Mathieu Poirier escreveu:
> > On Sun, 17 Jun 2018 at 23:10, Leo Yan <leo.yan@linaro.org> wrote:
> > >
> > > 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 on acme tree [1] on branch perf/core with
> > > latest commit: e238cf2e3d2e ("perf intel-pt: Fix packet decoding of CYC
> > > packets").  Also applied successfully this patch series on Linus tree
> > > on 4.18-rc1.
> > >
> > > 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
> > >
> > >
> > > 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(-)
> > 
> > Good day Arnaldo,
> > 
> > I am good with this set:
> > 
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > 
> > Please consider for inclusion in your tree if you are satisfied with the work.
> 
> I'll take a look and get it into perf/core, now I'm concentrating on
> perf/urgent work.

Gentle ping for this patch series.

I tested this patch series on perf/core branch with latest commit
87b40003b17a ("perf tests: Check that complex event name is parsed
correctly"), this patch series is still valid and can pass testing.

Could you pick up them?  If you want me to resend this patch series,
please let me know.

Thanks,
Leo Yan