From: Namhyung Kim <namhyung@kernel.org>
To: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: acme@kernel.org, jolsa@kernel.org, adrian.hunter@intel.com,
irogers@google.com, segher@kernel.crashing.org,
christophe.leroy@csgroup.eu, linux-kernel@vger.kernel.org,
linux-perf-users@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
akanksha@linux.ibm.com, maddy@linux.ibm.com, kjain@linux.ibm.com,
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: Tue, 2 Jul 2024 18:24:11 -0700 [thread overview]
Message-ID: <ZoSoO7IHKreTpkbt@google.com> (raw)
In-Reply-To: <20240701043430.66666-6-atrajeev@linux.vnet.ibm.com>
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
> + 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
>
WARNING: multiple messages have this Message-ID (diff)
From: Namhyung Kim <namhyung@kernel.org>
To: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: irogers@google.com, disgoel@linux.vnet.ibm.com,
maddy@linux.ibm.com, kjain@linux.ibm.com,
adrian.hunter@intel.com, christophe.leroy@csgroup.eu,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
acme@kernel.org, jolsa@kernel.org, akanksha@linux.ibm.com,
linuxppc-dev@lists.ozlabs.org
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: Tue, 2 Jul 2024 18:24:11 -0700 [thread overview]
Message-ID: <ZoSoO7IHKreTpkbt@google.com> (raw)
In-Reply-To: <20240701043430.66666-6-atrajeev@linux.vnet.ibm.com>
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
> + 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 1:24 UTC|newest]
Thread overview: 50+ 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 ` 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 ` 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 ` 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-01 4:34 ` Athira Rajeev
2024-07-03 0:09 ` Namhyung Kim
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-01 4:34 ` Athira Rajeev
2024-07-03 1:16 ` Namhyung Kim
2024-07-03 1:16 ` Namhyung Kim
2024-07-03 13:11 ` Athira Rajeev
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-01 4:34 ` Athira Rajeev
2024-07-03 1:24 ` Namhyung Kim [this message]
2024-07-03 1:24 ` Namhyung Kim
2024-07-03 11:52 ` Athira Rajeev
2024-07-03 11:52 ` Athira Rajeev
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 ` 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 ` 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 ` 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 ` 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 ` 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 ` 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 ` 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 ` 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 ` 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-01 4:34 ` Athira Rajeev
2024-07-03 1:41 ` Namhyung Kim
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-01 4:34 ` Athira Rajeev
2024-07-03 2:00 ` Namhyung Kim
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
2024-07-01 4:34 ` 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=ZoSoO7IHKreTpkbt@google.com \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=akanksha@linux.ibm.com \
--cc=atrajeev@linux.vnet.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=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 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.