From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Ian Rogers <irogers@google.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users <linux-perf-users@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
akanksha@linux.ibm.com, Madhavan Srinivasan <maddy@linux.ibm.com>,
Kajol Jain <kjain@linux.ibm.com>,
Disha Goel <disgoel@linux.vnet.ibm.com>
Subject: Re: [PATCH V5 05/17] tools/perf: Add support to capture and parse raw instruction in powerpc using dso__data_read_offset utility
Date: Wed, 3 Jul 2024 17:22:52 +0530 [thread overview]
Message-ID: <649287F9-2301-42B8-ABD9-E6B3DC6DA50F@linux.vnet.ibm.com> (raw)
In-Reply-To: <ZoSoO7IHKreTpkbt@google.com>
> On 3 Jul 2024, at 6:54 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Mon, Jul 01, 2024 at 10:04:18AM +0530, Athira Rajeev wrote:
>> Add support to capture and parse raw instruction in powerpc.
>> Currently, the perf tool infrastructure uses two ways to disassemble
>> and understand the instruction. One is objdump and other option is
>> via libcapstone.
>>
>> Currently, the perf tool infrastructure uses "--no-show-raw-insn" option
>> with "objdump" while disassemble. Example from powerpc with this option
>> for an instruction address is:
>>
>> Snippet from:
>> objdump --start-address=<address> --stop-address=<address> -d --no-show-raw-insn -C <vmlinux>
>>
>> c0000000010224b4: lwz r10,0(r9)
>>
>> This line "lwz r10,0(r9)" is parsed to extract instruction name,
>> registers names and offset. Also to find whether there is a memory
>> reference in the operands, "memory_ref_char" field of objdump is used.
>> For x86, "(" is used as memory_ref_char to tackle instructions of the
>> form "mov (%rax), %rcx".
>>
>> In case of powerpc, not all instructions using "(" are the only memory
>> instructions. Example, above instruction can also be of extended form (X
>> form) "lwzx r10,0,r19". Inorder to easy identify the instruction category
>> and extract the source/target registers, patch adds support to use raw
>> instruction for powerpc. Approach used is to read the raw instruction
>> directly from the DSO file using "dso__data_read_offset" utility which
>> is already implemented in perf infrastructure in "util/dso.c".
>>
>> Example:
>>
>> 38 01 81 e8 ld r4,312(r1)
>>
>> Here "38 01 81 e8" is the raw instruction representation. In powerpc,
>> this translates to instruction form: "ld RT,DS(RA)" and binary code
>> as:
>>
>> | 58 | RT | RA | DS | |
>> -------------------------------------
>> 0 6 11 16 30 31
>>
>> Function "symbol__disassemble_dso" is updated to read raw instruction
>> directly from DSO using dso__data_read_offset utility. In case of
>> above example, this captures:
>> line: 38 01 81 e8
>>
>> The above works well when perf report is invoked with only sort keys for
>> data type ie type and typeoff. Because there is no instruction level
>> annotation needed if only data type information is requested for. For
>> annotating sample, along with type and typeoff sort key, "sym" sort key
>> is also needed. And by default invoking just "perf report" uses sort key
>> "sym" that displays the symbol information.
>>
>> With approach changes in powerpc which first reads DSO for raw
>> instruction, "perf annotate" and "perf report" + a key breaks since
>> it doesn't do the instruction level disassembly.
>>
>> Snippet of result from perf report:
>>
>> Samples: 1K of event 'mem-loads', 4000 Hz, Event count (approx.): 937238
>> do_work /usr/bin/pmlogger [Percent: local period]
>> Percent│ ea230010
>> │ 3a550010
>> │ 3a600000
>>
>> │ 38f60001
>> │ 39490008
>> │ 42400438
>> 51.44 │ 81290008
>> │ 7d485378
>>
>> Here, raw instruction is displayed in the output instead of human
>> readable annotated form.
>>
>> One way to get the appropriate data is to specify "--objdump path", by
>> which code annotation will be done. But the default behaviour will be
>> changed. To fix this breakage, check if "sym" sort key is set. If so
>> fallback and use the libcapstone/objdump way of disassmbling the sample.
>>
>> With the changes and "perf report"
>>
>> Samples: 1K of event 'mem-loads', 4000 Hz, Event count (approx.): 937238
>> do_work /usr/bin/pmlogger [Percent: local period]
>> Percent│ ld r17,16(r3)
>> │ addi r18,r21,16
>> │ li r19,0
>>
>> │ 8b0: rldicl r10,r10,63,33
>> │ addi r10,r10,1
>> │ mtctr r10
>> │ ↓ b 8e4
>> │ 8c0: addi r7,r22,1
>> │ addi r10,r9,8
>> │ ↓ bdz d00
>> 51.44 │ lwz r9,8(r9)
>> │ mr r8,r10
>> │ cmpw r20,r9
>>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>> tools/perf/util/disasm.c | 101 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 101 insertions(+)
>>
>> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
>> index ddb861a0b043..e400dcab4029 100644
>> --- a/tools/perf/util/disasm.c
>> +++ b/tools/perf/util/disasm.c
>> @@ -25,6 +25,7 @@
>> #include "srcline.h"
>> #include "symbol.h"
>> #include "util.h"
>> +#include "sort.h"
>>
>> static regex_t file_lineno;
>>
>> @@ -1645,6 +1646,91 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
>> }
>> #endif
>>
>> +static int symbol__disassemble_dso(char *filename, struct symbol *sym,
>
> Again, I still think this should be named to symbol__disassemble_raw()
> because it only uses the raw binary codes. Using dso__data_read_offset
> is not important here and it's just a way of implementing it.
>
> Thanks,
> Namhyung
>
Hi Namhyung
Thanks for review comments
Sure, will change to symbol__disassemble_raw
Athira
>
>> + struct annotate_args *args)
>> +{
>> + struct annotation *notes = symbol__annotation(sym);
>> + struct map *map = args->ms.map;
>> + struct dso *dso = map__dso(map);
>> + u64 start = map__rip_2objdump(map, sym->start);
>> + u64 end = map__rip_2objdump(map, sym->end);
>> + u64 len = end - start;
>> + u64 offset;
>> + int i, count;
>> + u8 *buf = NULL;
>> + char disasm_buf[512];
>> + struct disasm_line *dl;
>> + u32 *line;
>> +
>> + /* Return if objdump is specified explicitly */
>> + if (args->options->objdump_path)
>> + return -1;
>> +
>> + pr_debug("Reading raw instruction from : %s using dso__data_read_offset\n", filename);
>> +
>> + buf = malloc(len);
>> + if (buf == NULL)
>> + goto err;
>> +
>> + count = dso__data_read_offset(dso, NULL, sym->start, buf, len);
>> +
>> + line = (u32 *)buf;
>> +
>> + if ((u64)count != len)
>> + goto err;
>> +
>> + /* add the function address and name */
>> + scnprintf(disasm_buf, sizeof(disasm_buf), "%#"PRIx64" <%s>:",
>> + start, sym->name);
>> +
>> + args->offset = -1;
>> + args->line = disasm_buf;
>> + args->line_nr = 0;
>> + args->fileloc = NULL;
>> + args->ms.sym = sym;
>> +
>> + dl = disasm_line__new(args);
>> + if (dl == NULL)
>> + goto err;
>> +
>> + annotation_line__add(&dl->al, ¬es->src->source);
>> +
>> + /* Each raw instruction is 4 byte */
>> + count = len/4;
>> +
>> + for (i = 0, offset = 0; i < count; i++) {
>> + args->offset = offset;
>> + sprintf(args->line, "%x", line[i]);
>> + dl = disasm_line__new(args);
>> + if (dl == NULL)
>> + goto err;
>> +
>> + annotation_line__add(&dl->al, ¬es->src->source);
>> + offset += 4;
>> + }
>> +
>> + /* It failed in the middle */
>> + if (offset != len) {
>> + struct list_head *list = ¬es->src->source;
>> +
>> + /* Discard all lines and fallback to objdump */
>> + while (!list_empty(list)) {
>> + dl = list_first_entry(list, struct disasm_line, al.node);
>> +
>> + list_del_init(&dl->al.node);
>> + disasm_line__free(dl);
>> + }
>> + count = -1;
>> + }
>> +
>> +out:
>> + free(buf);
>> + return count < 0 ? count : 0;
>> +
>> +err:
>> + count = -1;
>> + goto out;
>> +}
>> /*
>> * Possibly create a new version of line with tabs expanded. Returns the
>> * existing or new line, storage is updated if a new line is allocated. If
>> @@ -1769,6 +1855,21 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
>> strcpy(symfs_filename, tmp);
>> }
>>
>> + /*
>> + * For powerpc data type profiling, use the dso__data_read_offset
>> + * to read raw instruction directly and interpret the binary code
>> + * to understand instructions and register fields. For sort keys as
>> + * type and typeoff, disassemble to mnemonic notation is
>> + * not required in case of powerpc.
>> + */
>> + if (arch__is(args->arch, "powerpc")) {
>> + if (sort_order && !strstr(sort_order, "sym")) {
>> + err = symbol__disassemble_dso(symfs_filename, sym, args);
>> + if (err == 0)
>> + goto out_remove_tmp;
>> + }
>> + }
>> +
>> #ifdef HAVE_LIBCAPSTONE_SUPPORT
>> err = symbol__disassemble_capstone(symfs_filename, sym, args);
>> if (err == 0)
>> --
>> 2.43.0
next prev parent reply other threads:[~2024-07-03 11:57 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-01 4:34 [PATCH V5 00/17] Add data type profiling support for powerpc Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 01/17] tools/perf: Move the data structures related to register type to header file Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 02/17] tools/perf: Add "update_insn_state" callback function to handle arch specific instruction tracking Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 03/17] tools/perf: Update TYPE_STATE_MAX_REGS to include max of regs in powerpc Athira Rajeev
2024-07-03 0:09 ` Namhyung Kim
2024-07-01 4:34 ` [PATCH V5 04/17] tools/perf: Add disasm_line__parse to parse raw instruction for powerpc Athira Rajeev
2024-07-03 1:16 ` Namhyung Kim
2024-07-03 13:11 ` Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 05/17] tools/perf: Add support to capture and parse raw instruction in powerpc using dso__data_read_offset utility Athira Rajeev
2024-07-03 1:24 ` Namhyung Kim
2024-07-03 11:52 ` Athira Rajeev [this message]
2024-07-01 4:34 ` [PATCH V5 06/17] tools/perf: Update parameters for reg extract functions to use raw instruction on powerpc Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 07/17] tools/perf: Add parse function for memory instructions in powerpc Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 08/17] tools/perf: Add support to identify memory instructions of opcode 31 " Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 09/17] tools/perf: Add some of the arithmetic instructions to support instruction tracking " Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 10/17] tools/perf: Add more instructions for instruction tracking Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 11/17] tools/perf: Update instruction tracking for powerpc Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 12/17] tools/perf: Make capstone_init non-static so that it can be used during symbol disassemble Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 13/17] tools/perf: Use capstone_init and remove open_capstone_handle from disasm.c Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 14/17] tools/perf: Add support to use libcapstone in powerpc Athira Rajeev
2024-07-01 4:34 ` [PATCH V5 15/17] tools/perf: Add support to find global register variables using find_data_type_global_reg Athira Rajeev
2024-07-03 1:41 ` Namhyung Kim
2024-07-01 4:34 ` [PATCH V5 16/17] tools/perf: Add support for global_die to capture name of variable in case of register defined variable Athira Rajeev
2024-07-03 2:00 ` Namhyung Kim
2024-07-01 4:34 ` [PATCH V5 17/17] tools/perf: Set instruction name to be used with insn-stat when using raw instruction Athira Rajeev
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=649287F9-2301-42B8-ABD9-E6B3DC6DA50F@linux.vnet.ibm.com \
--to=atrajeev@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=akanksha@linux.ibm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=disgoel@linux.vnet.ibm.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kjain@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=namhyung@kernel.org \
--cc=segher@kernel.crashing.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).