2024-04-04 18:31:46

by Tanmay Jagdale

[permalink] [raw]
Subject: [PATCH V2 0/2] Fix Coresight instruction synthesis logic

When we use perf to catpure Coresight trace and generate instruction
trace using 'perf script', we get the following output:

# perf record -e cs_etm/@tmc_etr0/ -C 9 taskset -c 9 sleep 1
# perf script --itrace=i1ns --ns -Fcomm,tid,pid,time,cpu,event,ip,sym,addr,symoff,flags,callindent
..
perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed54 coresight_timeout+0x28
perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed58 coresight_timeout+0x2c
perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed5c coresight_timeout+0x30
perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed60 coresight_timeout+0x34
perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed7c coresight_timeout+0x50
perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed80 coresight_timeout+0x54
perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed84 coresight_timeout+0x58
perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aede4 coresight_timeout+0xb8
perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aede8 coresight_timeout+0xbc
perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aedec coresight_timeout+0xc0
perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aedf0 coresight_timeout+0xc4
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccec ete_sysreg_read+0x0
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf0 ete_sysreg_read+0x4
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf4 ete_sysreg_read+0x8
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf8 ete_sysreg_read+0xc
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccfc ete_sysreg_read+0x10
perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bcd00 ete_sysreg_read+0x14

This output has the following issues:
1. Non-branch instructions have mnemonics of branch instructions (Column 6)
2. Branch target address is missing (Column 7)

This patch fixes these issues by changing the logic of instruction syntehsis
for the Coresight trace queues.

Output after applying the patch:
...
perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed54 coresight_timeout+0x28
perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed58 coresight_timeout+0x2c
perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed5c coresight_timeout+0x30
perf 6111/6111 [008] 457.332794461: instructions: jmp ffffb305591aed7c ffffb305591aed60 coresight_timeout+0x34
perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed7c coresight_timeout+0x50
perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed80 coresight_timeout+0x54
perf 6111/6111 [008] 457.332794461: instructions: jcc ffffb305591aede4 ffffb305591aed84 coresight_timeout+0x58
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aede4 coresight_timeout+0xb8
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aede8 coresight_timeout+0xbc
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aedec coresight_timeout+0xc0
perf 6111/6111 [008] 457.332794462: instructions: call ffffb305591bccec ffffb305591aedf0 coresight_timeout+0xc4
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccec ete_sysreg_read+0x0
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf0 ete_sysreg_read+0x4
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf4 ete_sysreg_read+0x8
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf8 ete_sysreg_read+0xc
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccfc ete_sysreg_read+0x10
perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bcd00 ete_sysreg_read+0x14

Changes in V2
- Updated commit message of Patch 1
- As discussed in the previous version [1], there were differences in instruction
trace output before and after the patch. The timestamps for the instructions
were not in sync. Added a patch 2 which fixes this issue.

Changes in V1
- https://lkml.org/lkml/2023/6/23/912

[1] https://lkml.org/lkml/2023/6/28/506

Tanmay Jagdale (2):
perf: cs-etm: Fixes in instruction sample synthesis
perf: cs-etm: Store previous timestamp in packet queue

tools/perf/util/cs-etm.c | 49 +++++++++++++++++++++++++++++++---------
tools/perf/util/cs-etm.h | 1 +
2 files changed, 39 insertions(+), 11 deletions(-)

--
2.34.1



2024-04-04 18:32:59

by Tanmay Jagdale

[permalink] [raw]
Subject: [PATCH V2 2/2] perf: cs-etm: Store previous timestamp in packet queue

Since logic in cs_etm__sample is changed to synthesizing the previous
packet (tidq->prev_packet) instead of current packet (tidq->packet),
the first time this function is called, tidq->prev_packet is NULL
and we return without processing anything.

This is as expected but, in the first call, we would have a valid
timestamp (stored in tidq->packet_queue.cs_timestamp) which belongs
to tidq->packet. This would be lost due to no processing.

Losing this timestamp results in all the synthesized packets being
associated with the next timestamp and not their corresponding one.

To fix this, introduce a new variable (prev_cs_timestamp) to store the
queue's timestamp in cs_etm__sample(). When we start synthesizing the
prev_packet, use this cached value instead of the current timestamp
(cs_timestamp).

Signed-off-by: Tanmay Jagdale <[email protected]>
---
tools/perf/util/cs-etm.c | 17 +++++++++++++----
tools/perf/util/cs-etm.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 55db1932f785..d5072c16fcd8 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1459,13 +1459,15 @@ u64 cs_etm__convert_sample_time(struct cs_etm_queue *etmq, u64 cs_timestamp)
}

static inline u64 cs_etm__resolve_sample_time(struct cs_etm_queue *etmq,
- struct cs_etm_traceid_queue *tidq)
+ struct cs_etm_traceid_queue *tidq,
+ bool instruction_sample)
{
struct cs_etm_auxtrace *etm = etmq->etm;
struct cs_etm_packet_queue *packet_queue = &tidq->packet_queue;

if (!etm->timeless_decoding && etm->has_virtual_ts)
- return packet_queue->cs_timestamp;
+ return instruction_sample ? packet_queue->prev_cs_timestamp :
+ packet_queue->cs_timestamp;
else
return etm->latest_kernel_timestamp;
}
@@ -1484,7 +1486,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
event->sample.header.size = sizeof(struct perf_event_header);

/* Set time field based on etm auxtrace config. */
- sample.time = cs_etm__resolve_sample_time(etmq, tidq);
+ sample.time = cs_etm__resolve_sample_time(etmq, tidq, true);

sample.ip = addr;
sample.pid = thread__pid(tidq->thread);
@@ -1560,7 +1562,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
event->sample.header.size = sizeof(struct perf_event_header);

/* Set time field based on etm auxtrace config. */
- sample.time = cs_etm__resolve_sample_time(etmq, tidq);
+ sample.time = cs_etm__resolve_sample_time(etmq, tidq, false);

sample.ip = ip;
sample.pid = thread__pid(tidq->prev_packet_thread);
@@ -1849,6 +1851,13 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
}
}

+ /*
+ * Since we synthesize the prev_packet, store the current timestamp
+ * here in prev_cs_timestamp so that when we *actually* synthesize
+ * the prev_packet, we use this timestamp and not the future one.
+ */
+ tidq->packet_queue.prev_cs_timestamp = tidq->packet_queue.cs_timestamp;
+
cs_etm__packet_swap(etm, tidq);

return 0;
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
index 4696267a32f0..333eeb2c06a0 100644
--- a/tools/perf/util/cs-etm.h
+++ b/tools/perf/util/cs-etm.h
@@ -205,6 +205,7 @@ struct cs_etm_packet_queue {
u32 instr_count;
u64 cs_timestamp; /* Timestamp from trace data, converted to ns if possible */
u64 next_cs_timestamp;
+ u64 prev_cs_timestamp;
struct cs_etm_packet packet_buffer[CS_ETM_PACKET_MAX_BUFFER];
};

--
2.34.1


2024-04-25 18:55:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH V2 0/2] Fix Coresight instruction synthesis logic

On Thu, Apr 04, 2024 at 11:37:29PM +0530, Tanmay Jagdale wrote:
> When we use perf to catpure Coresight trace and generate instruction
> trace using 'perf script', we get the following output:
>
> # perf record -e cs_etm/@tmc_etr0/ -C 9 taskset -c 9 sleep 1
> # perf script --itrace=i1ns --ns -Fcomm,tid,pid,time,cpu,event,ip,sym,addr,symoff,flags,callindent

Applies cleanly, can some Coresight people review this and provide a
Reviewed-by?

Thanks!

- Arnaldo

⬢[acme@toolbox perf-tools-next]$ b4 am -ctsl --cc-trailers [email protected]
Grabbing thread from lore.kernel.org/all/[email protected]/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 3 messages in the thread
Checking attestation on all messages, may take a moment...
---
✓ [PATCH v2 1/2] perf: cs-etm: Fixes in instruction sample synthesis
+ Link: https://lore.kernel.org/r/[email protected]
+ Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
✓ [PATCH v2 2/2] perf: cs-etm: Store previous timestamp in packet queue
+ Link: https://lore.kernel.org/r/[email protected]
+ Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
✓ Signed: DKIM/marvell.com
---
Total patches: 2
---
Cover: ./v2_20240404_tanmay_fix_coresight_instruction_synthesis_logic.cover
Link: https://lore.kernel.org/r/[email protected]
Base: applies clean to current tree
git checkout -b v2_20240404_tanmay_marvell_com HEAD
git am ./v2_20240404_tanmay_fix_coresight_instruction_synthesis_logic.mbx
⬢[acme@toolbox perf-tools-next]$

2024-04-26 15:08:39

by James Clark

[permalink] [raw]
Subject: Re: [PATCH V2 0/2] Fix Coresight instruction synthesis logic



On 25/04/2024 19:54, Arnaldo Carvalho de Melo wrote:
> On Thu, Apr 04, 2024 at 11:37:29PM +0530, Tanmay Jagdale wrote:
>> When we use perf to catpure Coresight trace and generate instruction
>> trace using 'perf script', we get the following output:
>>
>> # perf record -e cs_etm/@tmc_etr0/ -C 9 taskset -c 9 sleep 1
>> # perf script --itrace=i1ns --ns -Fcomm,tid,pid,time,cpu,event,ip,sym,addr,symoff,flags,callindent
>
> Applies cleanly, can some Coresight people review this and provide a
> Reviewed-by?
>
> Thanks!
>

Reviewed-by: James Clark <[email protected]>