From: Leo Yan <leo.yan@linaro.org> To: Arnaldo Carvalho de Melo <acme@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mike Leach <mike.leach@linaro.org>, Robert Walker <robert.walker@arm.com>, Coresight ML <coresight@lists.linaro.org> Cc: Leo Yan <leo.yan@linaro.org> Subject: [PATCH v4 1/5] perf cs-etm: Refactor instruction size handling Date: Mon, 3 Feb 2020 10:07:12 +0800 [thread overview] Message-ID: <20200203020716.31832-2-leo.yan@linaro.org> (raw) In-Reply-To: <20200203020716.31832-1-leo.yan@linaro.org> cs-etm.c has several functions which need to know instruction size based on address, e.g. cs_etm__instr_addr() and cs_etm__copy_insn() two functions both calculate the instruction size separately with its duplicated code. Furthermore, adding new features later which might require to calculate instruction size as well. For this reason, this patch refactors the code to introduce a new function cs_etm__instr_size(), this function is central place to calculate the instruction size based on ISA type and instruction address. For a neat implementation, cs_etm__instr_addr() will always execute the loop without checking ISA type, this allows cs_etm__instr_size() and cs_etm__instr_addr() have no any duplicate code with each other and both functions are independent and can be changed separately without breaking anything. As a side effect, cs_etm__instr_addr() will do a few more iterations for A32/A64 instructions, this would be fine if consider perf is a tool running in the user space. Signed-off-by: Leo Yan <leo.yan@linaro.org> --- tools/perf/util/cs-etm.c | 48 +++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 720108bd8dba..cb6fcc2acca0 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -918,6 +918,26 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq, return ((instrBytes[1] & 0xF8) >= 0xE8) ? 4 : 2; } +static inline int cs_etm__instr_size(struct cs_etm_queue *etmq, + u8 trace_chan_id, + enum cs_etm_isa isa, + u64 addr) +{ + int insn_len; + + /* + * T32 instruction size might be 32-bit or 16-bit, decide by calling + * cs_etm__t32_instr_size(). + */ + if (isa == CS_ETM_ISA_T32) + insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, addr); + /* Otherwise, A64 and A32 instruction size are always 32-bit. */ + else + insn_len = 4; + + return insn_len; +} + static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet) { /* Returns 0 for the CS_ETM_DISCONTINUITY packet */ @@ -942,19 +962,15 @@ static inline u64 cs_etm__instr_addr(struct cs_etm_queue *etmq, const struct cs_etm_packet *packet, u64 offset) { - if (packet->isa == CS_ETM_ISA_T32) { - u64 addr = packet->start_addr; + u64 addr = packet->start_addr; - while (offset) { - addr += cs_etm__t32_instr_size(etmq, - trace_chan_id, addr); - offset--; - } - return addr; + while (offset) { + addr += cs_etm__instr_size(etmq, trace_chan_id, + packet->isa, addr); + offset--; } - /* Assume a 4 byte instruction size (A32/A64) */ - return packet->start_addr + offset * 4; + return addr; } static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq, @@ -1094,16 +1110,8 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq, return; } - /* - * T32 instruction size might be 32-bit or 16-bit, decide by calling - * cs_etm__t32_instr_size(). - */ - if (packet->isa == CS_ETM_ISA_T32) - sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, - sample->ip); - /* Otherwise, A64 and A32 instruction size are always 32-bit. */ - else - sample->insn_len = 4; + sample->insn_len = cs_etm__instr_size(etmq, trace_chan_id, + packet->isa, sample->ip); cs_etm__mem_access(etmq, trace_chan_id, sample->ip, sample->insn_len, (void *)sample->insn); -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan@linaro.org> To: Arnaldo Carvalho de Melo <acme@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mike Leach <mike.leach@linaro.org>, Robert Walker <robert.walker@arm.com>, Coresight ML <coresight@lists.linaro.org> Cc: Leo Yan <leo.yan@linaro.org> Subject: [PATCH v4 1/5] perf cs-etm: Refactor instruction size handling Date: Mon, 3 Feb 2020 10:07:12 +0800 [thread overview] Message-ID: <20200203020716.31832-2-leo.yan@linaro.org> (raw) In-Reply-To: <20200203020716.31832-1-leo.yan@linaro.org> cs-etm.c has several functions which need to know instruction size based on address, e.g. cs_etm__instr_addr() and cs_etm__copy_insn() two functions both calculate the instruction size separately with its duplicated code. Furthermore, adding new features later which might require to calculate instruction size as well. For this reason, this patch refactors the code to introduce a new function cs_etm__instr_size(), this function is central place to calculate the instruction size based on ISA type and instruction address. For a neat implementation, cs_etm__instr_addr() will always execute the loop without checking ISA type, this allows cs_etm__instr_size() and cs_etm__instr_addr() have no any duplicate code with each other and both functions are independent and can be changed separately without breaking anything. As a side effect, cs_etm__instr_addr() will do a few more iterations for A32/A64 instructions, this would be fine if consider perf is a tool running in the user space. Signed-off-by: Leo Yan <leo.yan@linaro.org> --- tools/perf/util/cs-etm.c | 48 +++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 720108bd8dba..cb6fcc2acca0 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -918,6 +918,26 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq, return ((instrBytes[1] & 0xF8) >= 0xE8) ? 4 : 2; } +static inline int cs_etm__instr_size(struct cs_etm_queue *etmq, + u8 trace_chan_id, + enum cs_etm_isa isa, + u64 addr) +{ + int insn_len; + + /* + * T32 instruction size might be 32-bit or 16-bit, decide by calling + * cs_etm__t32_instr_size(). + */ + if (isa == CS_ETM_ISA_T32) + insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, addr); + /* Otherwise, A64 and A32 instruction size are always 32-bit. */ + else + insn_len = 4; + + return insn_len; +} + static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet) { /* Returns 0 for the CS_ETM_DISCONTINUITY packet */ @@ -942,19 +962,15 @@ static inline u64 cs_etm__instr_addr(struct cs_etm_queue *etmq, const struct cs_etm_packet *packet, u64 offset) { - if (packet->isa == CS_ETM_ISA_T32) { - u64 addr = packet->start_addr; + u64 addr = packet->start_addr; - while (offset) { - addr += cs_etm__t32_instr_size(etmq, - trace_chan_id, addr); - offset--; - } - return addr; + while (offset) { + addr += cs_etm__instr_size(etmq, trace_chan_id, + packet->isa, addr); + offset--; } - /* Assume a 4 byte instruction size (A32/A64) */ - return packet->start_addr + offset * 4; + return addr; } static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq, @@ -1094,16 +1110,8 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq, return; } - /* - * T32 instruction size might be 32-bit or 16-bit, decide by calling - * cs_etm__t32_instr_size(). - */ - if (packet->isa == CS_ETM_ISA_T32) - sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, - sample->ip); - /* Otherwise, A64 and A32 instruction size are always 32-bit. */ - else - sample->insn_len = 4; + sample->insn_len = cs_etm__instr_size(etmq, trace_chan_id, + packet->isa, sample->ip); cs_etm__mem_access(etmq, trace_chan_id, sample->ip, sample->insn_len, (void *)sample->insn); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-03 2:08 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-03 2:07 [PATCH v4 0/5] perf cs-etm: Support thread stack and callchain Leo Yan 2020-02-03 2:07 ` Leo Yan 2020-02-03 2:07 ` Leo Yan [this message] 2020-02-03 2:07 ` [PATCH v4 1/5] perf cs-etm: Refactor instruction size handling Leo Yan 2020-02-06 12:36 ` Mike Leach 2020-02-06 12:36 ` Mike Leach 2020-02-10 5:46 ` Leo Yan 2020-02-10 5:46 ` Leo Yan 2020-02-03 2:07 ` [PATCH v4 2/5] perf cs-etm: Support thread stack Leo Yan 2020-02-03 2:07 ` Leo Yan 2020-02-03 2:07 ` [PATCH v4 3/5] perf cs-etm: Support branch filter Leo Yan 2020-02-03 2:07 ` Leo Yan 2020-02-03 2:07 ` [PATCH v4 4/5] perf cs-etm: Support callchain for instruction sample Leo Yan 2020-02-03 2:07 ` Leo Yan 2020-02-03 2:07 ` [PATCH v4 5/5] perf cs-etm: Synchronize instruction sample with the thread stack Leo Yan 2020-02-03 2:07 ` Leo Yan 2020-02-06 15:01 ` Mike Leach 2020-02-06 15:01 ` Mike Leach 2020-02-13 9:08 ` Leo Yan 2020-02-13 9:08 ` Leo Yan 2020-02-13 15:00 ` Mike Leach 2020-02-13 15:00 ` Mike Leach
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200203020716.31832-2-leo.yan@linaro.org \ --to=leo.yan@linaro.org \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=coresight@lists.linaro.org \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=robert.walker@arm.com \ --cc=suzuki.poulose@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.