Hi Mathieu,
On Thu, Dec 13, 2018 at 01:28:34PM -0700, Mathieu Poirier wrote:
On Tue, Dec 11, 2018 at 11:01:08PM +0800, Leo Yan wrote:
The perf sample data contains flags to indicate the hardware trace data is belonging to which type branch instruction, thus this can be used to print out the human readable string. Arm CoreSight ETM sample data is missed to set flags and it is always set to zeros, this results in perf tool skips to print string for instruction types.
This patch is to set branch instruction flags for instruction range packet,
Remove everything between here and your SOB. It is too long and you already added valuable comments to function cs_etm__set_sample_flags().
Will do.
mainly based on three fields which have been introduced in cs_etm_packet struct:
cs_etm_packet::last_instr_type cs_etm_packet::last_instr_subtype cs_etm_packet::last_instr_cond
Below is detailed implementation for set sample flags for branch instructions:
If the instruction is immediate branch but without link and return flag, we consider it as function internal branch; in fact the immediate branch also can be used to invoke the function entry, usually this is only used in assembly code to directly call a symbol and don't expect to return back; after reviewing kernel normal functions and user space programs, both of them are very seldom to use immediate branch for function call. On the other hand, if we want to decide the immediate branch is for function branch jumping or for function calling, we need to rely on the start address of next packet and check the symbol offset for the start address, this will introduce much complexity in the implementation. So for this version we simply consider immediate branch as function internal branch. Moreover, we rely on 'last_instr_cond' to decide if the branch instruction is a conditional branch or not.
If the instruction is immediate branch with link, it's instruction 'BL' and which is used for function call.
If the instruction is indirect branch with link, e.g BLR, we think it's a function call.
If the instruction is indirect branch and with subtype OCSD_S_INSTR_V7_IMPLIED_RET, the decoders gives the hint the function return for below cases related with A32/T32 instruction; set this branch flag as function return (Thanks for Al's suggestion).
BX R14 MOV PC, LR POP {…, PC} LDR PC, [SP], #offsetIf the instruction is indirect branch without link, this is corresponding to instruction 'BR', this instruction usually is used for dynamic link lib with below usage; so we think it's a return instruction.
0000000000000680 <.plt>: 680: a9bf7bf0 stp x16, x30, [sp, #-16]! 684: 90000090 adrp x16, 10000 <__FRAME_END__+0xf630> 688: f947fe11 ldr x17, [x16, #4088] 68c: 913fe210 add x16, x16, #0xff8 690: d61f0220 br x17For function return, ARMv8 introduces a dedicated instruction 'ret', which has flag of OCSD_S_INSTR_V8_RET.
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 | 81 ++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-)
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 0ffa7c5..516a0fb 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h @@ -49,6 +49,7 @@ struct cs_etm_packet { u32 last_instr_subtype; u8 last_instr_cond; int cpu;
- u32 flags;
Add this just below @last_instr_subtype so that types are grouped together.
Will do it.
}; struct cs_etm_queue; diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 27a374d..3ad0b87 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -13,6 +13,7 @@ #include <linux/types.h> #include <stdlib.h> +#include <opencsd/ocsd_if_types.h> #include "auxtrace.h" #include "color.h" @@ -719,7 +720,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, sample.stream_id = etmq->etm->instructions_id; sample.period = period; sample.cpu = etmq->packet->cpu;
- sample.flags = 0;
- sample.flags = etmq->prev_packet->flags; sample.insn_len = 1; sample.cpumode = event->sample.header.misc;
@@ -778,7 +779,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq) sample.stream_id = etmq->etm->branches_id; sample.period = 1; sample.cpu = etmq->packet->cpu;
- sample.flags = 0;
- sample.flags = etmq->prev_packet->flags; sample.cpumode = event->sample.header.misc;
/* @@ -1107,6 +1108,80 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq) return 0; } +static void cs_etm__set_sample_flags(struct cs_etm_queue *etmq) +{
- struct cs_etm_packet *packet = etmq->packet;
- packet->flags = 0;
- switch (packet->sample_type) {
- case CS_ETM_RANGE:
/** Immediate branch instruction without neither link nor* return flag, it's normal branch instruction within* the function.*/if (packet->last_instr_type == OCSD_INSTR_BR &&packet->last_instr_subtype == OCSD_S_INSTR_NONE) {packet->flags = PERF_IP_FLAG_BRANCH;if (packet->last_instr_cond)packet->flags |= PERF_IP_FLAG_CONDITIONAL;}/** Immediate branch instruction with link (e.g. BL), this is* branch instruction for function call.*/if (packet->last_instr_type == OCSD_INSTR_BR &&packet->last_instr_subtype == OCSD_S_INSTR_BR_LINK)packet->flags = PERF_IP_FLAG_BRANCH |PERF_IP_FLAG_CALL;/** Indirect branch instruction with link (e.g. BLR), this is* branch instruction for function call.*/if (packet->last_instr_type == OCSD_INSTR_BR_INDIRECT &&packet->last_instr_subtype == OCSD_S_INSTR_BR_LINK)packet->flags = PERF_IP_FLAG_BRANCH |PERF_IP_FLAG_CALL;/** Indirect branch instruction with subtype of* OCSD_S_INSTR_V7_IMPLIED_RET, this is explicit hint for* function return for A32/T32.*/if (packet->last_instr_type == OCSD_INSTR_BR_INDIRECT &&packet->last_instr_subtype == OCSD_S_INSTR_V7_IMPLIED_RET)packet->flags = PERF_IP_FLAG_BRANCH |PERF_IP_FLAG_RETURN;/** Indirect branch instruction without link (e.g. BR), usually* this is used for function return, especially for functions* within dynamic link lib.*/if (packet->last_instr_type == OCSD_INSTR_BR_INDIRECT &&packet->last_instr_subtype == OCSD_S_INSTR_NONE)packet->flags = PERF_IP_FLAG_BRANCH |PERF_IP_FLAG_RETURN;/* Return instruction for function return. */if (packet->last_instr_type == OCSD_INSTR_BR_INDIRECT &&packet->last_instr_subtype == OCSD_S_INSTR_V8_RET)packet->flags = PERF_IP_FLAG_BRANCH |PERF_IP_FLAG_RETURN;Many thanks for the comments - it is really clear and helps understand what you are doing.
Yeah, this also helps me for clear idea.
break;- case CS_ETM_DISCONTINUITY:
- case CS_ETM_EXCEPTION:
- case CS_ETM_EXCEPTION_RET:
- case CS_ETM_EMPTY:
- default:
break;- }
+}
static int cs_etm__run_decoder(struct cs_etm_queue *etmq) { struct cs_etm_auxtrace *etm = etmq->etm; @@ -1158,6 +1233,8 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq) */ break;
Intuitively one would be tempted to call cs_etm__set_sample_flags() after the switch statement but that won't work since packet addresses are swaped in cs_etm__sample(). As such I think it is worth adding a comment here stressing the importance of doing flag processing before the switch() statement.
Indeed. Will add comment for this.
And comments in other patches are also acked, will follow up in next series. Thanks a lot for reviewing and suggestions!
Leo Yan
cs_etm__set_sample_flags(etmq);switch (etmq->packet->sample_type) { case CS_ETM_RANGE: /*-- 2.7.4