* [PATCH v4 1/5] perf: build: introduce the libcapstone
2024-01-19 10:48 [PATCH v4 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
@ 2024-01-19 10:48 ` Changbin Du
2024-01-19 18:38 ` Adrian Hunter
2024-01-19 10:48 ` [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
` (3 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-19 10:48 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Changbin Du
Later we will use libcapstone to disassemble instructions of samples.
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
tools/build/Makefile.feature | 2 ++
tools/build/feature/Makefile | 4 ++++
tools/build/feature/test-all.c | 4 ++++
tools/build/feature/test-libcapstone.c | 11 +++++++++++
tools/perf/Makefile.config | 21 +++++++++++++++++++++
tools/perf/Makefile.perf | 3 +++
6 files changed, 45 insertions(+)
create mode 100644 tools/build/feature/test-libcapstone.c
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 934e2777a2db..23bee50aeb0f 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -86,6 +86,7 @@ FEATURE_TESTS_EXTRA := \
gtk2-infobar \
hello \
libbabeltrace \
+ libcapstone \
libbfd-liberty \
libbfd-liberty-z \
libopencsd \
@@ -133,6 +134,7 @@ FEATURE_DISPLAY ?= \
libcrypto \
libunwind \
libdw-dwarf-unwind \
+ libcapstone \
zlib \
lzma \
get_cpuid \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index dad79ede4e0a..d6eaade09694 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -53,6 +53,7 @@ FILES= \
test-timerfd.bin \
test-libdw-dwarf-unwind.bin \
test-libbabeltrace.bin \
+ test-libcapstone.bin \
test-compile-32.bin \
test-compile-x32.bin \
test-zlib.bin \
@@ -282,6 +283,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
$(OUTPUT)test-libbabeltrace.bin:
$(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
+$(OUTPUT)test-libcapstone.bin:
+ $(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
+
$(OUTPUT)test-compile-32.bin:
$(CC) -m32 -o $@ test-compile.c
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 6f4bf386a3b5..dd0a18c2ef8f 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -134,6 +134,10 @@
#undef main
#endif
+#define main main_test_libcapstone
+# include "test-libcapstone.c"
+#undef main
+
#define main main_test_lzma
# include "test-lzma.c"
#undef main
diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
new file mode 100644
index 000000000000..fbe8dba189e9
--- /dev/null
+++ b/tools/build/feature/test-libcapstone.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <capstone/capstone.h>
+
+int main(void)
+{
+ csh handle;
+
+ cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
+ return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index b3e6ed10f40c..7589725ad178 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -191,6 +191,15 @@ endif
FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
+# for linking with debug library, run like:
+# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
+ifdef LIBCAPSTONE_DIR
+ LIBCAPSTONE_CFLAGS := -I$(LIBCAPSTONE_DIR)/include
+ LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
+endif
+FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
+FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
+
ifdef LIBZSTD_DIR
LIBZSTD_CFLAGS := -I$(LIBZSTD_DIR)/lib
LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
@@ -1089,6 +1098,18 @@ ifndef NO_LIBBABELTRACE
endif
endif
+ifndef NO_CAPSTONE
+ $(call feature_check,libcapstone)
+ ifeq ($(feature-libcapstone), 1)
+ CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
+ LDFLAGS += $(LICAPSTONE_LDFLAGS)
+ EXTLIBS += -lcapstone
+ $(call detected,CONFIG_LIBCAPSTONE)
+ else
+ msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
+ endif
+endif
+
ifndef NO_AUXTRACE
ifeq ($(SRCARCH),x86)
ifeq ($(feature-get_cpuid), 0)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 058c9aecf608..236da4f39a63 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -84,6 +84,9 @@ include ../scripts/utilities.mak
# Define NO_LIBBABELTRACE if you do not want libbabeltrace support
# for CTF data format.
#
+# Define NO_CAPSTONE if you do not want libcapstone support
+# for disasm engine.
+#
# Define NO_LZMA if you do not want to support compressed (xz) kernel modules
#
# Define NO_AUXTRACE if you do not want AUX area tracing support
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 1/5] perf: build: introduce the libcapstone
2024-01-19 10:48 ` [PATCH v4 1/5] perf: build: introduce the libcapstone Changbin Du
@ 2024-01-19 18:38 ` Adrian Hunter
2024-01-20 7:23 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-19 18:38 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On 19/01/24 12:48, Changbin Du wrote:
> Later we will use libcapstone to disassemble instructions of samples.
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
> tools/build/Makefile.feature | 2 ++
> tools/build/feature/Makefile | 4 ++++
> tools/build/feature/test-all.c | 4 ++++
> tools/build/feature/test-libcapstone.c | 11 +++++++++++
> tools/perf/Makefile.config | 21 +++++++++++++++++++++
> tools/perf/Makefile.perf | 3 +++
tools/perf/tests/make needs updating also
> 6 files changed, 45 insertions(+)
> create mode 100644 tools/build/feature/test-libcapstone.c
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 934e2777a2db..23bee50aeb0f 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -86,6 +86,7 @@ FEATURE_TESTS_EXTRA := \
> gtk2-infobar \
> hello \
> libbabeltrace \
> + libcapstone \
> libbfd-liberty \
> libbfd-liberty-z \
> libopencsd \
> @@ -133,6 +134,7 @@ FEATURE_DISPLAY ?= \
> libcrypto \
> libunwind \
> libdw-dwarf-unwind \
> + libcapstone \
> zlib \
> lzma \
> get_cpuid \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index dad79ede4e0a..d6eaade09694 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -53,6 +53,7 @@ FILES= \
> test-timerfd.bin \
> test-libdw-dwarf-unwind.bin \
> test-libbabeltrace.bin \
> + test-libcapstone.bin \
> test-compile-32.bin \
> test-compile-x32.bin \
> test-zlib.bin \
> @@ -282,6 +283,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
> $(OUTPUT)test-libbabeltrace.bin:
> $(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
>
> +$(OUTPUT)test-libcapstone.bin:
> + $(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
> +
> $(OUTPUT)test-compile-32.bin:
> $(CC) -m32 -o $@ test-compile.c
>
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index 6f4bf386a3b5..dd0a18c2ef8f 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -134,6 +134,10 @@
> #undef main
> #endif
>
> +#define main main_test_libcapstone
> +# include "test-libcapstone.c"
> +#undef main
> +
> #define main main_test_lzma
> # include "test-lzma.c"
> #undef main
> diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
> new file mode 100644
> index 000000000000..fbe8dba189e9
> --- /dev/null
> +++ b/tools/build/feature/test-libcapstone.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <capstone/capstone.h>
> +
> +int main(void)
> +{
> + csh handle;
> +
> + cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
> + return 0;
> +}
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index b3e6ed10f40c..7589725ad178 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -191,6 +191,15 @@ endif
> FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
> FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
>
> +# for linking with debug library, run like:
> +# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
> +ifdef LIBCAPSTONE_DIR
> + LIBCAPSTONE_CFLAGS := -I$(LIBCAPSTONE_DIR)/include
> + LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
> +endif
> +FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
> +FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
> +
> ifdef LIBZSTD_DIR
> LIBZSTD_CFLAGS := -I$(LIBZSTD_DIR)/lib
> LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
> @@ -1089,6 +1098,18 @@ ifndef NO_LIBBABELTRACE
> endif
> endif
>
> +ifndef NO_CAPSTONE
> + $(call feature_check,libcapstone)
> + ifeq ($(feature-libcapstone), 1)
> + CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
> + LDFLAGS += $(LICAPSTONE_LDFLAGS)
> + EXTLIBS += -lcapstone
> + $(call detected,CONFIG_LIBCAPSTONE)
> + else
> + msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
> + endif
> +endif
> +
> ifndef NO_AUXTRACE
> ifeq ($(SRCARCH),x86)
> ifeq ($(feature-get_cpuid), 0)
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 058c9aecf608..236da4f39a63 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -84,6 +84,9 @@ include ../scripts/utilities.mak
> # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
> # for CTF data format.
> #
> +# Define NO_CAPSTONE if you do not want libcapstone support
> +# for disasm engine.
> +#
> # Define NO_LZMA if you do not want to support compressed (xz) kernel modules
> #
> # Define NO_AUXTRACE if you do not want AUX area tracing support
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 1/5] perf: build: introduce the libcapstone
2024-01-19 18:38 ` Adrian Hunter
@ 2024-01-20 7:23 ` Changbin Du
0 siblings, 0 replies; 22+ messages in thread
From: Changbin Du @ 2024-01-20 7:23 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Fri, Jan 19, 2024 at 08:38:40PM +0200, Adrian Hunter wrote:
> On 19/01/24 12:48, Changbin Du wrote:
> > Later we will use libcapstone to disassemble instructions of samples.
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> > tools/build/Makefile.feature | 2 ++
> > tools/build/feature/Makefile | 4 ++++
> > tools/build/feature/test-all.c | 4 ++++
> > tools/build/feature/test-libcapstone.c | 11 +++++++++++
> > tools/perf/Makefile.config | 21 +++++++++++++++++++++
> > tools/perf/Makefile.perf | 3 +++
>
> tools/perf/tests/make needs updating also
done. Added make_no_libcapstone target.
>
> > 6 files changed, 45 insertions(+)
> > create mode 100644 tools/build/feature/test-libcapstone.c
> >
> > diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> > index 934e2777a2db..23bee50aeb0f 100644
> > --- a/tools/build/Makefile.feature
> > +++ b/tools/build/Makefile.feature
> > @@ -86,6 +86,7 @@ FEATURE_TESTS_EXTRA := \
> > gtk2-infobar \
> > hello \
> > libbabeltrace \
> > + libcapstone \
> > libbfd-liberty \
> > libbfd-liberty-z \
> > libopencsd \
> > @@ -133,6 +134,7 @@ FEATURE_DISPLAY ?= \
> > libcrypto \
> > libunwind \
> > libdw-dwarf-unwind \
> > + libcapstone \
> > zlib \
> > lzma \
> > get_cpuid \
> > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > index dad79ede4e0a..d6eaade09694 100644
> > --- a/tools/build/feature/Makefile
> > +++ b/tools/build/feature/Makefile
> > @@ -53,6 +53,7 @@ FILES= \
> > test-timerfd.bin \
> > test-libdw-dwarf-unwind.bin \
> > test-libbabeltrace.bin \
> > + test-libcapstone.bin \
> > test-compile-32.bin \
> > test-compile-x32.bin \
> > test-zlib.bin \
> > @@ -282,6 +283,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
> > $(OUTPUT)test-libbabeltrace.bin:
> > $(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
> >
> > +$(OUTPUT)test-libcapstone.bin:
> > + $(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
> > +
> > $(OUTPUT)test-compile-32.bin:
> > $(CC) -m32 -o $@ test-compile.c
> >
> > diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> > index 6f4bf386a3b5..dd0a18c2ef8f 100644
> > --- a/tools/build/feature/test-all.c
> > +++ b/tools/build/feature/test-all.c
> > @@ -134,6 +134,10 @@
> > #undef main
> > #endif
> >
> > +#define main main_test_libcapstone
> > +# include "test-libcapstone.c"
> > +#undef main
> > +
> > #define main main_test_lzma
> > # include "test-lzma.c"
> > #undef main
> > diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
> > new file mode 100644
> > index 000000000000..fbe8dba189e9
> > --- /dev/null
> > +++ b/tools/build/feature/test-libcapstone.c
> > @@ -0,0 +1,11 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <capstone/capstone.h>
> > +
> > +int main(void)
> > +{
> > + csh handle;
> > +
> > + cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
> > + return 0;
> > +}
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index b3e6ed10f40c..7589725ad178 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -191,6 +191,15 @@ endif
> > FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
> > FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
> >
> > +# for linking with debug library, run like:
> > +# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
> > +ifdef LIBCAPSTONE_DIR
> > + LIBCAPSTONE_CFLAGS := -I$(LIBCAPSTONE_DIR)/include
> > + LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
> > +endif
> > +FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
> > +FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
> > +
> > ifdef LIBZSTD_DIR
> > LIBZSTD_CFLAGS := -I$(LIBZSTD_DIR)/lib
> > LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
> > @@ -1089,6 +1098,18 @@ ifndef NO_LIBBABELTRACE
> > endif
> > endif
> >
> > +ifndef NO_CAPSTONE
> > + $(call feature_check,libcapstone)
> > + ifeq ($(feature-libcapstone), 1)
> > + CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
> > + LDFLAGS += $(LICAPSTONE_LDFLAGS)
> > + EXTLIBS += -lcapstone
> > + $(call detected,CONFIG_LIBCAPSTONE)
> > + else
> > + msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
> > + endif
> > +endif
> > +
> > ifndef NO_AUXTRACE
> > ifeq ($(SRCARCH),x86)
> > ifeq ($(feature-get_cpuid), 0)
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 058c9aecf608..236da4f39a63 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -84,6 +84,9 @@ include ../scripts/utilities.mak
> > # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
> > # for CTF data format.
> > #
> > +# Define NO_CAPSTONE if you do not want libcapstone support
> > +# for disasm engine.
> > +#
> > # Define NO_LZMA if you do not want to support compressed (xz) kernel modules
> > #
> > # Define NO_AUXTRACE if you do not want AUX area tracing support
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions
2024-01-19 10:48 [PATCH v4 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
2024-01-19 10:48 ` [PATCH v4 1/5] perf: build: introduce the libcapstone Changbin Du
@ 2024-01-19 10:48 ` Changbin Du
2024-01-19 18:39 ` Adrian Hunter
2024-01-19 10:48 ` [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
` (2 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-19 10:48 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Changbin Du
Currently, the instructions of samples are shown as raw hex strings
which are hard to read. x86 has a special option '--xed' to disassemble
the hex string via intel XED tool.
Here we use capstone as our disassembler engine to give more friendly
instructions. We select libcapstone because capstone can provide more
insn details. Perf will fallback to raw instructions if libcapstone is
not available.
The advantages compared to XED tool:
* Support arm, arm64, x86-32, x86_64 (more could be supported),
xed only for x86_64.
* Immediate address operands are shown as symbol+offs.
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
tools/perf/builtin-script.c | 8 +--
tools/perf/util/Build | 1 +
tools/perf/util/print_insn.c | 122 +++++++++++++++++++++++++++++++++++
tools/perf/util/print_insn.h | 14 ++++
4 files changed, 140 insertions(+), 5 deletions(-)
create mode 100644 tools/perf/util/print_insn.c
create mode 100644 tools/perf/util/print_insn.h
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b1f57401ff23..4817a37f16e2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -34,6 +34,7 @@
#include "util/event.h"
#include "ui/ui.h"
#include "print_binary.h"
+#include "print_insn.h"
#include "archinsn.h"
#include <linux/bitmap.h>
#include <linux/kernel.h>
@@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
if (PRINT_FIELD(INSNLEN))
printed += fprintf(fp, " ilen: %d", sample->insn_len);
if (PRINT_FIELD(INSN) && sample->insn_len) {
- int i;
-
- printed += fprintf(fp, " insn:");
- for (i = 0; i < sample->insn_len; i++)
- printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
+ printed += fprintf(fp, " insn: ");
+ printed += sample__fprintf_insn_raw(sample, fp);
}
if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 988473bf907a..c33aab53d8dd 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -32,6 +32,7 @@ perf-y += perf_regs.o
perf-y += perf-regs-arch/
perf-y += path.o
perf-y += print_binary.o
+perf-y += print_insn.o
perf-y += rlimit.o
perf-y += argv_split.o
perf-y += rbtree.o
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
new file mode 100644
index 000000000000..162be4856f79
--- /dev/null
+++ b/tools/perf/util/print_insn.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Instruction binary disassembler based on capstone.
+ *
+ * Author(s): Changbin Du <changbin.du@huawei.com>
+ */
+#include "print_insn.h"
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+#include "util/debug.h"
+#include "util/symbol.h"
+#include "machine.h"
+
+size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
+{
+ int printed = 0;
+
+ for (int i = 0; i < sample->insn_len; i++)
+ printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
+ return printed;
+}
+
+#ifdef HAVE_LIBCAPSTONE_SUPPORT
+#include <capstone/capstone.h>
+
+static int capstone_init(struct machine *machine, csh *cs_handle)
+{
+ cs_arch arch;
+ cs_mode mode;
+
+ if (machine__is(machine, "x86_64")) {
+ arch = CS_ARCH_X86;
+ mode = CS_MODE_64;
+ } else if (machine__normalized_is(machine, "x86")) {
+ arch = CS_ARCH_X86;
+ mode = CS_MODE_32;
+ } else if (machine__normalized_is(machine, "arm64")) {
+ arch = CS_ARCH_ARM64;
+ mode = CS_MODE_ARM;
+ } else if (machine__normalized_is(machine, "arm")) {
+ arch = CS_ARCH_ARM;
+ mode = CS_MODE_ARM + CS_MODE_V8;
+ } else if (machine__normalized_is(machine, "s390")) {
+ arch = CS_ARCH_SYSZ;
+ mode = CS_MODE_BIG_ENDIAN;
+ } else {
+ return -1;
+ }
+
+ if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
+ pr_warning_once("cs_open failed\n");
+ return -1;
+ }
+
+ cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
+ if (machine__normalized_is(machine, "x86"))
+ cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
+
+ return 0;
+}
+
+static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
+ cs_insn *insn, FILE *fp)
+{
+ struct addr_location al;
+ size_t printed = 0;
+
+ if (insn->detail && insn->detail->x86.op_count == 1) {
+ cs_x86_op *op = &insn->detail->x86.operands[0];
+
+ addr_location__init(&al);
+
+ if (op->type == X86_OP_IMM &&
+ thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
+ printed += fprintf(fp, "%s ", insn[0].mnemonic);
+ printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
+ return printed;
+ }
+ }
+
+ printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+ return printed;
+}
+
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
+ struct machine *machine, FILE *fp)
+{
+ static csh cs_handle;
+ cs_insn *insn;
+ size_t count;
+ size_t printed = 0;
+ int ret;
+
+ ret = capstone_init(machine, &cs_handle);
+ if (ret < 0) {
+ /* fallback */
+ return sample__fprintf_insn_raw(sample, fp);
+ }
+
+ count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
+ sample->ip, 1, &insn);
+ if (count > 0) {
+ if (machine__normalized_is(machine, "x86"))
+ printed += print_insn_x86(sample, thread, &insn[0], fp);
+ else
+ printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+ cs_free(insn, count);
+ } else {
+ printed += fprintf(fp, "illegal instruction");
+ }
+
+ cs_close(&cs_handle);
+ return printed;
+}
+#else
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
+ struct machine *machine __maybe_unused, FILE *fp)
+{
+ return sample__fprintf_insn_raw(sample, fp);
+}
+#endif
diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
new file mode 100644
index 000000000000..af8fa5d01fb7
--- /dev/null
+++ b/tools/perf/util/print_insn.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef PERF_PRINT_ISNS_H
+#define PERF_PRINT_ISNS_H
+
+#include <stddef.h>
+#include <stdio.h>
+#include "event.h"
+#include "util/thread.h"
+
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
+ struct machine *machine, FILE *fp);
+size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
+
+#endif /* PERF_PRINT_ISNS_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions
2024-01-19 10:48 ` [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
@ 2024-01-19 18:39 ` Adrian Hunter
2024-01-20 9:13 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-19 18:39 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On 19/01/24 12:48, Changbin Du wrote:
> Currently, the instructions of samples are shown as raw hex strings
> which are hard to read. x86 has a special option '--xed' to disassemble
> the hex string via intel XED tool.
>
> Here we use capstone as our disassembler engine to give more friendly
> instructions. We select libcapstone because capstone can provide more
> insn details. Perf will fallback to raw instructions if libcapstone is
> not available.
>
> The advantages compared to XED tool:
> * Support arm, arm64, x86-32, x86_64 (more could be supported),
> xed only for x86_64.
> * Immediate address operands are shown as symbol+offs.
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
> tools/perf/builtin-script.c | 8 +--
> tools/perf/util/Build | 1 +
> tools/perf/util/print_insn.c | 122 +++++++++++++++++++++++++++++++++++
> tools/perf/util/print_insn.h | 14 ++++
> 4 files changed, 140 insertions(+), 5 deletions(-)
> create mode 100644 tools/perf/util/print_insn.c
> create mode 100644 tools/perf/util/print_insn.h
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b1f57401ff23..4817a37f16e2 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -34,6 +34,7 @@
> #include "util/event.h"
> #include "ui/ui.h"
> #include "print_binary.h"
> +#include "print_insn.h"
> #include "archinsn.h"
> #include <linux/bitmap.h>
> #include <linux/kernel.h>
> @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> if (PRINT_FIELD(INSNLEN))
> printed += fprintf(fp, " ilen: %d", sample->insn_len);
> if (PRINT_FIELD(INSN) && sample->insn_len) {
> - int i;
> -
> - printed += fprintf(fp, " insn:");
> - for (i = 0; i < sample->insn_len; i++)
> - printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> + printed += fprintf(fp, " insn: ");
"insn:" seems unnecessary. Also xed prints 2 tabs, which
helps line up the output. Perhaps 1 tab and 2 spaces is
enough.
> + printed += sample__fprintf_insn_raw(sample, fp);
> }
> if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 988473bf907a..c33aab53d8dd 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -32,6 +32,7 @@ perf-y += perf_regs.o
> perf-y += perf-regs-arch/
> perf-y += path.o
> perf-y += print_binary.o
> +perf-y += print_insn.o
> perf-y += rlimit.o
> perf-y += argv_split.o
> perf-y += rbtree.o
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> new file mode 100644
> index 000000000000..162be4856f79
> --- /dev/null
> +++ b/tools/perf/util/print_insn.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Instruction binary disassembler based on capstone.
> + *
> + * Author(s): Changbin Du <changbin.du@huawei.com>
> + */
> +#include "print_insn.h"
Please put with the other non-system includes
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include "util/debug.h"
util/ not needed
> +#include "util/symbol.h"
util/ not needed
> +#include "machine.h"
> +
> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
> +{
> + int printed = 0;
> +
> + for (int i = 0; i < sample->insn_len; i++)
> + printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
Why change this to put a space on the end?
> + return printed;
> +}
> +
> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> +#include <capstone/capstone.h>
> +
> +static int capstone_init(struct machine *machine, csh *cs_handle)
> +{
> + cs_arch arch;
> + cs_mode mode;
> +
> + if (machine__is(machine, "x86_64")) {
> + arch = CS_ARCH_X86;
> + mode = CS_MODE_64;
> + } else if (machine__normalized_is(machine, "x86")) {
> + arch = CS_ARCH_X86;
> + mode = CS_MODE_32;
> + } else if (machine__normalized_is(machine, "arm64")) {
> + arch = CS_ARCH_ARM64;
> + mode = CS_MODE_ARM;
> + } else if (machine__normalized_is(machine, "arm")) {
> + arch = CS_ARCH_ARM;
> + mode = CS_MODE_ARM + CS_MODE_V8;
> + } else if (machine__normalized_is(machine, "s390")) {
> + arch = CS_ARCH_SYSZ;
> + mode = CS_MODE_BIG_ENDIAN;
> + } else {
> + return -1;
> + }
> +
> + if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
> + pr_warning_once("cs_open failed\n");
> + return -1;
> + }
> +
> + cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
That's only needed for x86 isn't it
> + if (machine__normalized_is(machine, "x86"))
> + cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
Why? Could use a comment.
> +
> + return 0;
> +}
> +
> +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> + cs_insn *insn, FILE *fp)
> +{
> + struct addr_location al;
> + size_t printed = 0;
> +
> + if (insn->detail && insn->detail->x86.op_count == 1) {
> + cs_x86_op *op = &insn->detail->x86.operands[0];
> +
> + addr_location__init(&al);
Missing addr_location__exit()
> +
> + if (op->type == X86_OP_IMM &&
> + thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> + printed += fprintf(fp, "%s ", insn[0].mnemonic);
> + printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> + return printed;
> + }
> + }
> +
> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> + return printed;
> +}
> +
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> + struct machine *machine, FILE *fp)
> +{
> + static csh cs_handle;
Why static?
> + cs_insn *insn;
> + size_t count;
> + size_t printed = 0;
> + int ret;
> +
> + ret = capstone_init(machine, &cs_handle);
Does this really need to be done every time?
> + if (ret < 0) {
> + /* fallback */
> + return sample__fprintf_insn_raw(sample, fp);
> + }
> +
> + count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> + sample->ip, 1, &insn);
> + if (count > 0) {
> + if (machine__normalized_is(machine, "x86"))
> + printed += print_insn_x86(sample, thread, &insn[0], fp);
> + else
> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> + cs_free(insn, count);
> + } else {
> + printed += fprintf(fp, "illegal instruction");
> + }
> +
> + cs_close(&cs_handle);
> + return printed;
> +}
> +#else
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
> + struct machine *machine __maybe_unused, FILE *fp)
> +{
> + return sample__fprintf_insn_raw(sample, fp);
> +}
> +#endif
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> new file mode 100644
> index 000000000000..af8fa5d01fb7
> --- /dev/null
> +++ b/tools/perf/util/print_insn.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef PERF_PRINT_ISNS_H
Here and elsewhere
ISNS -> INSN
> +#define PERF_PRINT_ISNS_H
> +
> +#include <stddef.h>
> +#include <stdio.h>
> +#include "event.h"
> +#include "util/thread.h"
Instead of including event.h and thread.h, just forward declare:
struct perf_sample;
struct thread;
struct machine;
> +
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> + struct machine *machine, FILE *fp);
> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> +
> +#endif /* PERF_PRINT_ISNS_H */
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions
2024-01-19 18:39 ` Adrian Hunter
@ 2024-01-20 9:13 ` Changbin Du
2024-01-22 8:24 ` Adrian Hunter
0 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-20 9:13 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Fri, Jan 19, 2024 at 08:39:19PM +0200, Adrian Hunter wrote:
> On 19/01/24 12:48, Changbin Du wrote:
> > Currently, the instructions of samples are shown as raw hex strings
> > which are hard to read. x86 has a special option '--xed' to disassemble
> > the hex string via intel XED tool.
> >
> > Here we use capstone as our disassembler engine to give more friendly
> > instructions. We select libcapstone because capstone can provide more
> > insn details. Perf will fallback to raw instructions if libcapstone is
> > not available.
> >
> > The advantages compared to XED tool:
> > * Support arm, arm64, x86-32, x86_64 (more could be supported),
> > xed only for x86_64.
> > * Immediate address operands are shown as symbol+offs.
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> > tools/perf/builtin-script.c | 8 +--
> > tools/perf/util/Build | 1 +
> > tools/perf/util/print_insn.c | 122 +++++++++++++++++++++++++++++++++++
> > tools/perf/util/print_insn.h | 14 ++++
> > 4 files changed, 140 insertions(+), 5 deletions(-)
> > create mode 100644 tools/perf/util/print_insn.c
> > create mode 100644 tools/perf/util/print_insn.h
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index b1f57401ff23..4817a37f16e2 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -34,6 +34,7 @@
> > #include "util/event.h"
> > #include "ui/ui.h"
> > #include "print_binary.h"
> > +#include "print_insn.h"
> > #include "archinsn.h"
> > #include <linux/bitmap.h>
> > #include <linux/kernel.h>
> > @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> > if (PRINT_FIELD(INSNLEN))
> > printed += fprintf(fp, " ilen: %d", sample->insn_len);
> > if (PRINT_FIELD(INSN) && sample->insn_len) {
> > - int i;
> > -
> > - printed += fprintf(fp, " insn:");
> > - for (i = 0; i < sample->insn_len; i++)
> > - printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> > + printed += fprintf(fp, " insn: ");
>
> "insn:" seems unnecessary. Also xed prints 2 tabs, which
> helps line up the output. Perhaps 1 tab and 2 spaces is
> enough.
>
The "insn:" is used by xed. So it can not be removed if we preserve xed
function.
For 'insn' field, I keep the original output format.
For 'disasm' field, we can line up the output. I changed to 2 tabs and removed
'insn:'.
> > + printed += sample__fprintf_insn_raw(sample, fp);
> > }
> > if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> > printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 988473bf907a..c33aab53d8dd 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -32,6 +32,7 @@ perf-y += perf_regs.o
> > perf-y += perf-regs-arch/
> > perf-y += path.o
> > perf-y += print_binary.o
> > +perf-y += print_insn.o
> > perf-y += rlimit.o
> > perf-y += argv_split.o
> > perf-y += rbtree.o
> > diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> > new file mode 100644
> > index 000000000000..162be4856f79
> > --- /dev/null
> > +++ b/tools/perf/util/print_insn.c
> > @@ -0,0 +1,122 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Instruction binary disassembler based on capstone.
> > + *
> > + * Author(s): Changbin Du <changbin.du@huawei.com>
> > + */
> > +#include "print_insn.h"
>
> Please put with the other non-system includes
>
done.
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include "util/debug.h"
>
> util/ not needed
>
done.
> > +#include "util/symbol.h"
>
> util/ not needed
>
done.
> > +#include "machine.h"
> > +
> > +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
> > +{
> > + int printed = 0;
> > +
> > + for (int i = 0; i < sample->insn_len; i++)
> > + printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
>
> Why change this to put a space on the end?
>
Removed the tailing space.
> > + return printed;
> > +}
> > +
> > +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> > +#include <capstone/capstone.h>
> > +
> > +static int capstone_init(struct machine *machine, csh *cs_handle)
> > +{
> > + cs_arch arch;
> > + cs_mode mode;
> > +
> > + if (machine__is(machine, "x86_64")) {
> > + arch = CS_ARCH_X86;
> > + mode = CS_MODE_64;
> > + } else if (machine__normalized_is(machine, "x86")) {
> > + arch = CS_ARCH_X86;
> > + mode = CS_MODE_32;
> > + } else if (machine__normalized_is(machine, "arm64")) {
> > + arch = CS_ARCH_ARM64;
> > + mode = CS_MODE_ARM;
> > + } else if (machine__normalized_is(machine, "arm")) {
> > + arch = CS_ARCH_ARM;
> > + mode = CS_MODE_ARM + CS_MODE_V8;
> > + } else if (machine__normalized_is(machine, "s390")) {
> > + arch = CS_ARCH_SYSZ;
> > + mode = CS_MODE_BIG_ENDIAN;
> > + } else {
> > + return -1;
> > + }
> > +
> > + if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
> > + pr_warning_once("cs_open failed\n");
> > + return -1;
> > + }
> > +
> > + cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
>
> That's only needed for x86 isn't it
>
Moved into below branch.
> > + if (machine__normalized_is(machine, "x86"))
> > + cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
>
> Why? Could use a comment.
>
/*
* Resolving address oprands to symbols is implemented
* on x86 by investigating instruction details.
*/
> > +
> > + return 0;
> > +}
> > +
> > +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> > + cs_insn *insn, FILE *fp)
> > +{
> > + struct addr_location al;
> > + size_t printed = 0;
> > +
> > + if (insn->detail && insn->detail->x86.op_count == 1) {
> > + cs_x86_op *op = &insn->detail->x86.operands[0];
> > +
> > + addr_location__init(&al);
>
> Missing addr_location__exit()
>
Fixed.
> > +
> > + if (op->type == X86_OP_IMM &&
> > + thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> > + printed += fprintf(fp, "%s ", insn[0].mnemonic);
> > + printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> > + return printed;
> > + }
> > + }
> > +
> > + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> > + return printed;
> > +}
> > +
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> > + struct machine *machine, FILE *fp)
> > +{
> > + static csh cs_handle;
>
> Why static?
>
Removed. See below.
> > + cs_insn *insn;
> > + size_t count;
> > + size_t printed = 0;
> > + int ret;
> > +
> > + ret = capstone_init(machine, &cs_handle);
>
> Does this really need to be done every time?
>
Only need to init once exactly. The problem is I cannot find a appropriate
place to do the initiation.
I tried to initiate on first call but we still need a global mutex to be
initiated.
So finally I fallback to initiate every time. The redundant initiation is
acceptable per my test.
> > + if (ret < 0) {
> > + /* fallback */
> > + return sample__fprintf_insn_raw(sample, fp);
> > + }
> > +
> > + count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> > + sample->ip, 1, &insn);
> > + if (count > 0) {
> > + if (machine__normalized_is(machine, "x86"))
> > + printed += print_insn_x86(sample, thread, &insn[0], fp);
> > + else
> > + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> > + cs_free(insn, count);
> > + } else {
> > + printed += fprintf(fp, "illegal instruction");
> > + }
> > +
> > + cs_close(&cs_handle);
> > + return printed;
> > +}
> > +#else
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
> > + struct machine *machine __maybe_unused, FILE *fp)
> > +{
> > + return sample__fprintf_insn_raw(sample, fp);
> > +}
> > +#endif
> > diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> > new file mode 100644
> > index 000000000000..af8fa5d01fb7
> > --- /dev/null
> > +++ b/tools/perf/util/print_insn.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef PERF_PRINT_ISNS_H
>
> Here and elsewhere
>
> ISNS -> INSN
>
fixed.
> > +#define PERF_PRINT_ISNS_H
> > +
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +#include "event.h"
> > +#include "util/thread.h"
>
> Instead of including event.h and thread.h, just forward declare:
>
> struct perf_sample;
> struct thread;
> struct machine;
>
Why forward declaration?
> > +
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> > + struct machine *machine, FILE *fp);
> > +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> > +
> > +#endif /* PERF_PRINT_ISNS_H */
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions
2024-01-20 9:13 ` Changbin Du
@ 2024-01-22 8:24 ` Adrian Hunter
2024-01-22 8:42 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-22 8:24 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On 20/01/24 11:13, Changbin Du wrote:
> On Fri, Jan 19, 2024 at 08:39:19PM +0200, Adrian Hunter wrote:
>> On 19/01/24 12:48, Changbin Du wrote:
>>> Currently, the instructions of samples are shown as raw hex strings
>>> which are hard to read. x86 has a special option '--xed' to disassemble
>>> the hex string via intel XED tool.
>>>
>>> Here we use capstone as our disassembler engine to give more friendly
>>> instructions. We select libcapstone because capstone can provide more
>>> insn details. Perf will fallback to raw instructions if libcapstone is
>>> not available.
>>>
>>> The advantages compared to XED tool:
>>> * Support arm, arm64, x86-32, x86_64 (more could be supported),
>>> xed only for x86_64.
>>> * Immediate address operands are shown as symbol+offs.
>>>
>>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
>>> ---
>>> tools/perf/builtin-script.c | 8 +--
>>> tools/perf/util/Build | 1 +
>>> tools/perf/util/print_insn.c | 122 +++++++++++++++++++++++++++++++++++
>>> tools/perf/util/print_insn.h | 14 ++++
>>> 4 files changed, 140 insertions(+), 5 deletions(-)
>>> create mode 100644 tools/perf/util/print_insn.c
>>> create mode 100644 tools/perf/util/print_insn.h
>>>
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index b1f57401ff23..4817a37f16e2 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -34,6 +34,7 @@
>>> #include "util/event.h"
>>> #include "ui/ui.h"
>>> #include "print_binary.h"
>>> +#include "print_insn.h"
>>> #include "archinsn.h"
>>> #include <linux/bitmap.h>
>>> #include <linux/kernel.h>
>>> @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>>> if (PRINT_FIELD(INSNLEN))
>>> printed += fprintf(fp, " ilen: %d", sample->insn_len);
>>> if (PRINT_FIELD(INSN) && sample->insn_len) {
>>> - int i;
>>> -
>>> - printed += fprintf(fp, " insn:");
>>> - for (i = 0; i < sample->insn_len; i++)
>>> - printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
>>> + printed += fprintf(fp, " insn: ");
>>
>> "insn:" seems unnecessary. Also xed prints 2 tabs, which
>> helps line up the output. Perhaps 1 tab and 2 spaces is
>> enough.
>>
> The "insn:" is used by xed. So it can not be removed if we preserve xed
> function.
I got mixed up - it is patch 3 that the comment is meant for.
>
> For 'insn' field, I keep the original output format.
> For 'disasm' field, we can line up the output. I changed to 2 tabs and removed
> 'insn:'.
That is better, thanks!
>
>>> + printed += sample__fprintf_insn_raw(sample, fp);
>>> }
>>> if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>>> printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
>>> index 988473bf907a..c33aab53d8dd 100644
>>> --- a/tools/perf/util/Build
>>> +++ b/tools/perf/util/Build
>>> @@ -32,6 +32,7 @@ perf-y += perf_regs.o
>>> perf-y += perf-regs-arch/
>>> perf-y += path.o
>>> perf-y += print_binary.o
>>> +perf-y += print_insn.o
>>> perf-y += rlimit.o
>>> perf-y += argv_split.o
>>> perf-y += rbtree.o
>>> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
>>> new file mode 100644
>>> index 000000000000..162be4856f79
>>> --- /dev/null
>>> +++ b/tools/perf/util/print_insn.c
>>> @@ -0,0 +1,122 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Instruction binary disassembler based on capstone.
>>> + *
>>> + * Author(s): Changbin Du <changbin.du@huawei.com>
>>> + */
>>> +#include "print_insn.h"
>>
>> Please put with the other non-system includes
>>
> done.
>
>>> +#include <stdlib.h>
>>> +#include <string.h>
>>> +#include <stdbool.h>
>>> +#include "util/debug.h"
>>
>> util/ not needed
>>
> done.
>
>>> +#include "util/symbol.h"
>>
>> util/ not needed
>>
> done.
>
>>> +#include "machine.h"
>>> +
>>> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>>> +{
>>> + int printed = 0;
>>> +
>>> + for (int i = 0; i < sample->insn_len; i++)
>>> + printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
>>
>> Why change this to put a space on the end?
>>
> Removed the tailing space.
>
>>> + return printed;
>>> +}
>>> +
>>> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
>>> +#include <capstone/capstone.h>
>>> +
>>> +static int capstone_init(struct machine *machine, csh *cs_handle)
>>> +{
>>> + cs_arch arch;
>>> + cs_mode mode;
>>> +
>>> + if (machine__is(machine, "x86_64")) {
>>> + arch = CS_ARCH_X86;
>>> + mode = CS_MODE_64;
>>> + } else if (machine__normalized_is(machine, "x86")) {
>>> + arch = CS_ARCH_X86;
>>> + mode = CS_MODE_32;
>>> + } else if (machine__normalized_is(machine, "arm64")) {
>>> + arch = CS_ARCH_ARM64;
>>> + mode = CS_MODE_ARM;
>>> + } else if (machine__normalized_is(machine, "arm")) {
>>> + arch = CS_ARCH_ARM;
>>> + mode = CS_MODE_ARM + CS_MODE_V8;
>>> + } else if (machine__normalized_is(machine, "s390")) {
>>> + arch = CS_ARCH_SYSZ;
>>> + mode = CS_MODE_BIG_ENDIAN;
>>> + } else {
>>> + return -1;
>>> + }
>>> +
>>> + if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
>>> + pr_warning_once("cs_open failed\n");
>>> + return -1;
>>> + }
>>> +
>>> + cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
>>
>> That's only needed for x86 isn't it
>>
> Moved into below branch.
>
>>> + if (machine__normalized_is(machine, "x86"))
>>> + cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
>>
>> Why? Could use a comment.
>>
> /*
> * Resolving address oprands to symbols is implemented
oprands -> operands
> * on x86 by investigating instruction details.
> */
>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>>> + cs_insn *insn, FILE *fp)
>>> +{
>>> + struct addr_location al;
>>> + size_t printed = 0;
>>> +
>>> + if (insn->detail && insn->detail->x86.op_count == 1) {
>>> + cs_x86_op *op = &insn->detail->x86.operands[0];
>>> +
>>> + addr_location__init(&al);
>>
>> Missing addr_location__exit()
>>
> Fixed.
>
>>> +
>>> + if (op->type == X86_OP_IMM &&
>>> + thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
>>> + printed += fprintf(fp, "%s ", insn[0].mnemonic);
>>> + printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
>>> + return printed;
>>> + }
>>> + }
>>> +
>>> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
>>> + return printed;
>>> +}
>>> +
>>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
>>> + struct machine *machine, FILE *fp)
>>> +{
>>> + static csh cs_handle;
>>
>> Why static?
>>
> Removed. See below.
>
>>> + cs_insn *insn;
>>> + size_t count;
>>> + size_t printed = 0;
>>> + int ret;
>>> +
>>> + ret = capstone_init(machine, &cs_handle);
>>
>> Does this really need to be done every time?
>>
> Only need to init once exactly. The problem is I cannot find a appropriate
> place to do the initiation.
>
> I tried to initiate on first call but we still need a global mutex to be
> initiated.
>
> So finally I fallback to initiate every time. The redundant initiation is
> acceptable per my test.
OK, could add a FIXME
>
>>> + if (ret < 0) {
>>> + /* fallback */
>>> + return sample__fprintf_insn_raw(sample, fp);
>>> + }
>>> +
>>> + count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
>>> + sample->ip, 1, &insn);
>>> + if (count > 0) {
>>> + if (machine__normalized_is(machine, "x86"))
>>> + printed += print_insn_x86(sample, thread, &insn[0], fp);
>>> + else
>>> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
>>> + cs_free(insn, count);
>>> + } else {
>>> + printed += fprintf(fp, "illegal instruction");
>>> + }
>>> +
>>> + cs_close(&cs_handle);
>>> + return printed;
>>> +}
>>> +#else
>>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
>>> + struct machine *machine __maybe_unused, FILE *fp)
>>> +{
>>> + return sample__fprintf_insn_raw(sample, fp);
>>> +}
>>> +#endif
>>> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
>>> new file mode 100644
>>> index 000000000000..af8fa5d01fb7
>>> --- /dev/null
>>> +++ b/tools/perf/util/print_insn.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef PERF_PRINT_ISNS_H
>>
>> Here and elsewhere
>>
>> ISNS -> INSN
>>
> fixed.
>
>>> +#define PERF_PRINT_ISNS_H
>>> +
>>> +#include <stddef.h>
>>> +#include <stdio.h>
>>> +#include "event.h"
>>> +#include "util/thread.h"
>>
>> Instead of including event.h and thread.h, just forward declare:
>>
>> struct perf_sample;
>> struct thread;
>> struct machine;
>>
> Why forward declaration?
That is how it is done in perf tools. It avoids unnecessary dependency
among headers.
>
>>> +
>>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
>>> + struct machine *machine, FILE *fp);
>>> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
>>> +
>>> +#endif /* PERF_PRINT_ISNS_H */
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions
2024-01-22 8:24 ` Adrian Hunter
@ 2024-01-22 8:42 ` Changbin Du
0 siblings, 0 replies; 22+ messages in thread
From: Changbin Du @ 2024-01-22 8:42 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Mon, Jan 22, 2024 at 10:24:21AM +0200, Adrian Hunter wrote:
> On 20/01/24 11:13, Changbin Du wrote:
> > On Fri, Jan 19, 2024 at 08:39:19PM +0200, Adrian Hunter wrote:
> >> On 19/01/24 12:48, Changbin Du wrote:
> >>> Currently, the instructions of samples are shown as raw hex strings
> >>> which are hard to read. x86 has a special option '--xed' to disassemble
> >>> the hex string via intel XED tool.
> >>>
> >>> Here we use capstone as our disassembler engine to give more friendly
> >>> instructions. We select libcapstone because capstone can provide more
> >>> insn details. Perf will fallback to raw instructions if libcapstone is
> >>> not available.
> >>>
> >>> The advantages compared to XED tool:
> >>> * Support arm, arm64, x86-32, x86_64 (more could be supported),
> >>> xed only for x86_64.
> >>> * Immediate address operands are shown as symbol+offs.
> >>>
> >>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> >>> ---
> >>> tools/perf/builtin-script.c | 8 +--
> >>> tools/perf/util/Build | 1 +
> >>> tools/perf/util/print_insn.c | 122 +++++++++++++++++++++++++++++++++++
> >>> tools/perf/util/print_insn.h | 14 ++++
> >>> 4 files changed, 140 insertions(+), 5 deletions(-)
> >>> create mode 100644 tools/perf/util/print_insn.c
> >>> create mode 100644 tools/perf/util/print_insn.h
> >>>
> >>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> >>> index b1f57401ff23..4817a37f16e2 100644
> >>> --- a/tools/perf/builtin-script.c
> >>> +++ b/tools/perf/builtin-script.c
> >>> @@ -34,6 +34,7 @@
> >>> #include "util/event.h"
> >>> #include "ui/ui.h"
> >>> #include "print_binary.h"
> >>> +#include "print_insn.h"
> >>> #include "archinsn.h"
> >>> #include <linux/bitmap.h>
> >>> #include <linux/kernel.h>
> >>> @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> >>> if (PRINT_FIELD(INSNLEN))
> >>> printed += fprintf(fp, " ilen: %d", sample->insn_len);
> >>> if (PRINT_FIELD(INSN) && sample->insn_len) {
> >>> - int i;
> >>> -
> >>> - printed += fprintf(fp, " insn:");
> >>> - for (i = 0; i < sample->insn_len; i++)
> >>> - printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> >>> + printed += fprintf(fp, " insn: ");
> >>
> >> "insn:" seems unnecessary. Also xed prints 2 tabs, which
> >> helps line up the output. Perhaps 1 tab and 2 spaces is
> >> enough.
> >>
> > The "insn:" is used by xed. So it can not be removed if we preserve xed
> > function.
>
> I got mixed up - it is patch 3 that the comment is meant for.
>
> >
> > For 'insn' field, I keep the original output format.
> > For 'disasm' field, we can line up the output. I changed to 2 tabs and removed
> > 'insn:'.
>
> That is better, thanks!
>
> >
> >>> + printed += sample__fprintf_insn_raw(sample, fp);
> >>> }
> >>> if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> >>> printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> >>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> >>> index 988473bf907a..c33aab53d8dd 100644
> >>> --- a/tools/perf/util/Build
> >>> +++ b/tools/perf/util/Build
> >>> @@ -32,6 +32,7 @@ perf-y += perf_regs.o
> >>> perf-y += perf-regs-arch/
> >>> perf-y += path.o
> >>> perf-y += print_binary.o
> >>> +perf-y += print_insn.o
> >>> perf-y += rlimit.o
> >>> perf-y += argv_split.o
> >>> perf-y += rbtree.o
> >>> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> >>> new file mode 100644
> >>> index 000000000000..162be4856f79
> >>> --- /dev/null
> >>> +++ b/tools/perf/util/print_insn.c
> >>> @@ -0,0 +1,122 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Instruction binary disassembler based on capstone.
> >>> + *
> >>> + * Author(s): Changbin Du <changbin.du@huawei.com>
> >>> + */
> >>> +#include "print_insn.h"
> >>
> >> Please put with the other non-system includes
> >>
> > done.
> >
> >>> +#include <stdlib.h>
> >>> +#include <string.h>
> >>> +#include <stdbool.h>
> >>> +#include "util/debug.h"
> >>
> >> util/ not needed
> >>
> > done.
> >
> >>> +#include "util/symbol.h"
> >>
> >> util/ not needed
> >>
> > done.
> >
> >>> +#include "machine.h"
> >>> +
> >>> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
> >>> +{
> >>> + int printed = 0;
> >>> +
> >>> + for (int i = 0; i < sample->insn_len; i++)
> >>> + printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
> >>
> >> Why change this to put a space on the end?
> >>
> > Removed the tailing space.
> >
> >>> + return printed;
> >>> +}
> >>> +
> >>> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> >>> +#include <capstone/capstone.h>
> >>> +
> >>> +static int capstone_init(struct machine *machine, csh *cs_handle)
> >>> +{
> >>> + cs_arch arch;
> >>> + cs_mode mode;
> >>> +
> >>> + if (machine__is(machine, "x86_64")) {
> >>> + arch = CS_ARCH_X86;
> >>> + mode = CS_MODE_64;
> >>> + } else if (machine__normalized_is(machine, "x86")) {
> >>> + arch = CS_ARCH_X86;
> >>> + mode = CS_MODE_32;
> >>> + } else if (machine__normalized_is(machine, "arm64")) {
> >>> + arch = CS_ARCH_ARM64;
> >>> + mode = CS_MODE_ARM;
> >>> + } else if (machine__normalized_is(machine, "arm")) {
> >>> + arch = CS_ARCH_ARM;
> >>> + mode = CS_MODE_ARM + CS_MODE_V8;
> >>> + } else if (machine__normalized_is(machine, "s390")) {
> >>> + arch = CS_ARCH_SYSZ;
> >>> + mode = CS_MODE_BIG_ENDIAN;
> >>> + } else {
> >>> + return -1;
> >>> + }
> >>> +
> >>> + if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
> >>> + pr_warning_once("cs_open failed\n");
> >>> + return -1;
> >>> + }
> >>> +
> >>> + cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
> >>
> >> That's only needed for x86 isn't it
> >>
> > Moved into below branch.
> >
> >>> + if (machine__normalized_is(machine, "x86"))
> >>> + cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
> >>
> >> Why? Could use a comment.
> >>
> > /*
> > * Resolving address oprands to symbols is implemented
>
> oprands -> operands
Fixed, thanks!
>
> > * on x86 by investigating instruction details.
> > */
> >
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> >>> + cs_insn *insn, FILE *fp)
> >>> +{
> >>> + struct addr_location al;
> >>> + size_t printed = 0;
> >>> +
> >>> + if (insn->detail && insn->detail->x86.op_count == 1) {
> >>> + cs_x86_op *op = &insn->detail->x86.operands[0];
> >>> +
> >>> + addr_location__init(&al);
> >>
> >> Missing addr_location__exit()
> >>
> > Fixed.
> >
> >>> +
> >>> + if (op->type == X86_OP_IMM &&
> >>> + thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> >>> + printed += fprintf(fp, "%s ", insn[0].mnemonic);
> >>> + printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> >>> + return printed;
> >>> + }
> >>> + }
> >>> +
> >>> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> >>> + return printed;
> >>> +}
> >>> +
> >>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> >>> + struct machine *machine, FILE *fp)
> >>> +{
> >>> + static csh cs_handle;
> >>
> >> Why static?
> >>
> > Removed. See below.
> >
> >>> + cs_insn *insn;
> >>> + size_t count;
> >>> + size_t printed = 0;
> >>> + int ret;
> >>> +
> >>> + ret = capstone_init(machine, &cs_handle);
> >>
> >> Does this really need to be done every time?
> >>
> > Only need to init once exactly. The problem is I cannot find a appropriate
> > place to do the initiation.
> >
> > I tried to initiate on first call but we still need a global mutex to be
> > initiated.
> >
> > So finally I fallback to initiate every time. The redundant initiation is
> > acceptable per my test.
>
> OK, could add a FIXME
>
I added a TODO instead.
/* TODO: Try to initiate capstone only once but need a proper place. */
> >
> >>> + if (ret < 0) {
> >>> + /* fallback */
> >>> + return sample__fprintf_insn_raw(sample, fp);
> >>> + }
> >>> +
> >>> + count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> >>> + sample->ip, 1, &insn);
> >>> + if (count > 0) {
> >>> + if (machine__normalized_is(machine, "x86"))
> >>> + printed += print_insn_x86(sample, thread, &insn[0], fp);
> >>> + else
> >>> + printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> >>> + cs_free(insn, count);
> >>> + } else {
> >>> + printed += fprintf(fp, "illegal instruction");
> >>> + }
> >>> +
> >>> + cs_close(&cs_handle);
> >>> + return printed;
> >>> +}
> >>> +#else
> >>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
> >>> + struct machine *machine __maybe_unused, FILE *fp)
> >>> +{
> >>> + return sample__fprintf_insn_raw(sample, fp);
> >>> +}
> >>> +#endif
> >>> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> >>> new file mode 100644
> >>> index 000000000000..af8fa5d01fb7
> >>> --- /dev/null
> >>> +++ b/tools/perf/util/print_insn.h
> >>> @@ -0,0 +1,14 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +#ifndef PERF_PRINT_ISNS_H
> >>
> >> Here and elsewhere
> >>
> >> ISNS -> INSN
> >>
> > fixed.
> >
> >>> +#define PERF_PRINT_ISNS_H
> >>> +
> >>> +#include <stddef.h>
> >>> +#include <stdio.h>
> >>> +#include "event.h"
> >>> +#include "util/thread.h"
> >>
> >> Instead of including event.h and thread.h, just forward declare:
> >>
> >> struct perf_sample;
> >> struct thread;
> >> struct machine;
> >>
> > Why forward declaration?
>
> That is how it is done in perf tools. It avoids unnecessary dependency
> among headers.
ok, done.
>
> >
> >>> +
> >>> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> >>> + struct machine *machine, FILE *fp);
> >>> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> >>> +
> >>> +#endif /* PERF_PRINT_ISNS_H */
> >>
> >
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-19 10:48 [PATCH v4 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
2024-01-19 10:48 ` [PATCH v4 1/5] perf: build: introduce the libcapstone Changbin Du
2024-01-19 10:48 ` [PATCH v4 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
@ 2024-01-19 10:48 ` Changbin Du
2024-01-19 18:39 ` Adrian Hunter
2024-01-19 10:48 ` [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
2024-01-19 10:48 ` [PATCH v4 5/5] perf: script: prefer capstone to XED Changbin Du
4 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-19 10:48 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Changbin Du
In addition to the 'insn' field, this adds a new field 'disasm' to
display mnemonic instructions instead of the raw code.
$ sudo perf script -F +disasm
perf-exec 1443864 [006] 2275506.209848: psb: psb offs: 0 0 [unknown] ([unknown])
perf-exec 1443864 [006] 2275506.209848: cbr: cbr: 41 freq: 4100 MHz (114%) 0 [unknown] ([unknown])
ls 1443864 [006] 2275506.209905: 1 branches:uH: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
ls 1443864 [006] 2275506.209908: 1 branches:uH: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
tools/perf/Documentation/perf-script.txt | 7 ++++---
tools/perf/builtin-script.c | 8 +++++++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index ff9a52e44688..fc79167c6bf8 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -132,9 +132,10 @@ OPTIONS
Comma separated list of fields to print. Options are:
comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
- brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
- phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
- machine_pid, vcpu, cgroup, retire_lat.
+ brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
+ insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
+ code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
+
Field list can be prepended with the type, trace, sw or hw,
to indicate to which event type the field list applies.
e.g., -F sw:comm,tid,time,ip,sym and -F trace:time,cpu,trace
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4817a37f16e2..12d886694f6c 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -135,6 +135,7 @@ enum perf_output_field {
PERF_OUTPUT_CGROUP = 1ULL << 39,
PERF_OUTPUT_RETIRE_LAT = 1ULL << 40,
PERF_OUTPUT_DSOFF = 1ULL << 41,
+ PERF_OUTPUT_DISASM = 1ULL << 42,
};
struct perf_script {
@@ -190,6 +191,7 @@ struct output_option {
{.str = "bpf-output", .field = PERF_OUTPUT_BPF_OUTPUT},
{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
{.str = "insn", .field = PERF_OUTPUT_INSN},
+ {.str = "disasm", .field = PERF_OUTPUT_DISASM},
{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
@@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
printed += fprintf(fp, " insn: ");
printed += sample__fprintf_insn_raw(sample, fp);
}
+ if (PRINT_FIELD(DISASM) && sample->insn_len) {
+ printed += fprintf(fp, " insn: ");
+ printed += sample__fprintf_insn(sample, thread, machine, fp);
+ }
if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
@@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
"Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
"addr,symoff,srcline,period,iregs,uregs,brstack,"
"brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
- "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
+ "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
"phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
"code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
parse_output_fields),
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-19 10:48 ` [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
@ 2024-01-19 18:39 ` Adrian Hunter
2024-01-20 7:40 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-19 18:39 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On 19/01/24 12:48, Changbin Du wrote:
> In addition to the 'insn' field, this adds a new field 'disasm' to
> display mnemonic instructions instead of the raw code.
>
> $ sudo perf script -F +disasm
> perf-exec 1443864 [006] 2275506.209848: psb: psb offs: 0 0 [unknown] ([unknown])
> perf-exec 1443864 [006] 2275506.209848: cbr: cbr: 41 freq: 4100 MHz (114%) 0 [unknown] ([unknown])
> ls 1443864 [006] 2275506.209905: 1 branches:uH: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
> ls 1443864 [006] 2275506.209908: 1 branches:uH: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
> tools/perf/Documentation/perf-script.txt | 7 ++++---
> tools/perf/builtin-script.c | 8 +++++++-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index ff9a52e44688..fc79167c6bf8 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -132,9 +132,10 @@ OPTIONS
> Comma separated list of fields to print. Options are:
> comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
> srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> - brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> - phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> - machine_pid, vcpu, cgroup, retire_lat.
> + brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> + insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> + code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> +
Further down, there are explanations for insn and insnlen. disasm
could be added there.
> Field list can be prepended with the type, trace, sw or hw,
> to indicate to which event type the field list applies.
> e.g., -F sw:comm,tid,time,ip,sym and -F trace:time,cpu,trace
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 4817a37f16e2..12d886694f6c 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -135,6 +135,7 @@ enum perf_output_field {
> PERF_OUTPUT_CGROUP = 1ULL << 39,
> PERF_OUTPUT_RETIRE_LAT = 1ULL << 40,
> PERF_OUTPUT_DSOFF = 1ULL << 41,
> + PERF_OUTPUT_DISASM = 1ULL << 42,
> };
>
> struct perf_script {
> @@ -190,6 +191,7 @@ struct output_option {
> {.str = "bpf-output", .field = PERF_OUTPUT_BPF_OUTPUT},
> {.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
> {.str = "insn", .field = PERF_OUTPUT_INSN},
> + {.str = "disasm", .field = PERF_OUTPUT_DISASM},
> {.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
> {.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
> {.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> printed += fprintf(fp, " insn: ");
> printed += sample__fprintf_insn_raw(sample, fp);
> }
> + if (PRINT_FIELD(DISASM) && sample->insn_len) {
> + printed += fprintf(fp, " insn: ");
> + printed += sample__fprintf_insn(sample, thread, machine, fp);
> + }
> if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>
> @@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
> "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
> "addr,symoff,srcline,period,iregs,uregs,brstack,"
> "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
> - "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
> + "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
> "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
> "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
> parse_output_fields),
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-19 18:39 ` Adrian Hunter
@ 2024-01-20 7:40 ` Changbin Du
2024-01-22 9:59 ` Adrian Hunter
0 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-20 7:40 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Fri, Jan 19, 2024 at 08:39:36PM +0200, Adrian Hunter wrote:
> On 19/01/24 12:48, Changbin Du wrote:
> > In addition to the 'insn' field, this adds a new field 'disasm' to
> > display mnemonic instructions instead of the raw code.
> >
> > $ sudo perf script -F +disasm
> > perf-exec 1443864 [006] 2275506.209848: psb: psb offs: 0 0 [unknown] ([unknown])
> > perf-exec 1443864 [006] 2275506.209848: cbr: cbr: 41 freq: 4100 MHz (114%) 0 [unknown] ([unknown])
> > ls 1443864 [006] 2275506.209905: 1 branches:uH: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
> > ls 1443864 [006] 2275506.209908: 1 branches:uH: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> > tools/perf/Documentation/perf-script.txt | 7 ++++---
> > tools/perf/builtin-script.c | 8 +++++++-
> > 2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index ff9a52e44688..fc79167c6bf8 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -132,9 +132,10 @@ OPTIONS
> > Comma separated list of fields to print. Options are:
> > comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
> > srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> > - brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> > - phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> > - machine_pid, vcpu, cgroup, retire_lat.
> > + brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> > + insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> > + code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> > +
>
> Further down, there are explanations for insn and insnlen. disasm
> could be added there.
>
Updated as:
When doing instruction trace decoding, insn, disasm and insnlen give the
instruction bytes, disassembled instructions and the instruction length
of the current instruction respectively.
> > Field list can be prepended with the type, trace, sw or hw,
> > to indicate to which event type the field list applies.
> > e.g., -F sw:comm,tid,time,ip,sym and -F trace:time,cpu,trace
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 4817a37f16e2..12d886694f6c 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -135,6 +135,7 @@ enum perf_output_field {
> > PERF_OUTPUT_CGROUP = 1ULL << 39,
> > PERF_OUTPUT_RETIRE_LAT = 1ULL << 40,
> > PERF_OUTPUT_DSOFF = 1ULL << 41,
> > + PERF_OUTPUT_DISASM = 1ULL << 42,
> > };
> >
> > struct perf_script {
> > @@ -190,6 +191,7 @@ struct output_option {
> > {.str = "bpf-output", .field = PERF_OUTPUT_BPF_OUTPUT},
> > {.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
> > {.str = "insn", .field = PERF_OUTPUT_INSN},
> > + {.str = "disasm", .field = PERF_OUTPUT_DISASM},
> > {.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
> > {.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
> > {.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> > @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> > printed += fprintf(fp, " insn: ");
> > printed += sample__fprintf_insn_raw(sample, fp);
> > }
> > + if (PRINT_FIELD(DISASM) && sample->insn_len) {
> > + printed += fprintf(fp, " insn: ");
> > + printed += sample__fprintf_insn(sample, thread, machine, fp);
> > + }
> > if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> > printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> >
> > @@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
> > "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
> > "addr,symoff,srcline,period,iregs,uregs,brstack,"
> > "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
> > - "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
> > + "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
> > "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
> > "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
> > parse_output_fields),
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-20 7:40 ` Changbin Du
@ 2024-01-22 9:59 ` Adrian Hunter
2024-01-22 10:46 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-22 9:59 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On 20/01/24 09:40, Changbin Du wrote:
> On Fri, Jan 19, 2024 at 08:39:36PM +0200, Adrian Hunter wrote:
>> On 19/01/24 12:48, Changbin Du wrote:
>>> In addition to the 'insn' field, this adds a new field 'disasm' to
>>> display mnemonic instructions instead of the raw code.
>>>
>>> $ sudo perf script -F +disasm
>>> perf-exec 1443864 [006] 2275506.209848: psb: psb offs: 0 0 [unknown] ([unknown])
>>> perf-exec 1443864 [006] 2275506.209848: cbr: cbr: 41 freq: 4100 MHz (114%) 0 [unknown] ([unknown])
>>> ls 1443864 [006] 2275506.209905: 1 branches:uH: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
>>> ls 1443864 [006] 2275506.209908: 1 branches:uH: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
>>>
>>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
>>> ---
>>> tools/perf/Documentation/perf-script.txt | 7 ++++---
>>> tools/perf/builtin-script.c | 8 +++++++-
>>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
>>> index ff9a52e44688..fc79167c6bf8 100644
>>> --- a/tools/perf/Documentation/perf-script.txt
>>> +++ b/tools/perf/Documentation/perf-script.txt
>>> @@ -132,9 +132,10 @@ OPTIONS
>>> Comma separated list of fields to print. Options are:
>>> comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
>>> srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
>>> - brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
>>> - phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
>>> - machine_pid, vcpu, cgroup, retire_lat.
>>> + brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
>>> + insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
>>> + code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
>>> +
>>
>> Further down, there are explanations for insn and insnlen. disasm
>> could be added there.
>>
> Updated as:
>
> When doing instruction trace decoding, insn, disasm and insnlen give the
> instruction bytes, disassembled instructions and the instruction length
> of the current instruction respectively.
I wondered about mentioning that disasm needs perf to be compiled with
disassembler support, but with a permissive license it seems likely
that libcapstone support would generally be built into perf, so that
should be fine.
>
>>> Field list can be prepended with the type, trace, sw or hw,
>>> to indicate to which event type the field list applies.
>>> e.g., -F sw:comm,tid,time,ip,sym and -F trace:time,cpu,trace
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index 4817a37f16e2..12d886694f6c 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -135,6 +135,7 @@ enum perf_output_field {
>>> PERF_OUTPUT_CGROUP = 1ULL << 39,
>>> PERF_OUTPUT_RETIRE_LAT = 1ULL << 40,
>>> PERF_OUTPUT_DSOFF = 1ULL << 41,
>>> + PERF_OUTPUT_DISASM = 1ULL << 42,
>>> };
>>>
>>> struct perf_script {
>>> @@ -190,6 +191,7 @@ struct output_option {
>>> {.str = "bpf-output", .field = PERF_OUTPUT_BPF_OUTPUT},
>>> {.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
>>> {.str = "insn", .field = PERF_OUTPUT_INSN},
>>> + {.str = "disasm", .field = PERF_OUTPUT_DISASM},
>>> {.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
>>> {.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
>>> {.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
>>> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>>> printed += fprintf(fp, " insn: ");
>>> printed += sample__fprintf_insn_raw(sample, fp);
>>> }
>>> + if (PRINT_FIELD(DISASM) && sample->insn_len) {
>>> + printed += fprintf(fp, " insn: ");
>>> + printed += sample__fprintf_insn(sample, thread, machine, fp);
>>> + }
>>> if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>>> printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>>>
>>> @@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
>>> "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
>>> "addr,symoff,srcline,period,iregs,uregs,brstack,"
>>> "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
>>> - "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
>>> + "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
>>> "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
>>> "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
>>> parse_output_fields),
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-22 9:59 ` Adrian Hunter
@ 2024-01-22 10:46 ` Changbin Du
2024-01-22 13:41 ` Andi Kleen
0 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-22 10:46 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Mon, Jan 22, 2024 at 11:59:13AM +0200, Adrian Hunter wrote:
> On 20/01/24 09:40, Changbin Du wrote:
> > On Fri, Jan 19, 2024 at 08:39:36PM +0200, Adrian Hunter wrote:
> >> On 19/01/24 12:48, Changbin Du wrote:
> >>> In addition to the 'insn' field, this adds a new field 'disasm' to
> >>> display mnemonic instructions instead of the raw code.
> >>>
> >>> $ sudo perf script -F +disasm
> >>> perf-exec 1443864 [006] 2275506.209848: psb: psb offs: 0 0 [unknown] ([unknown])
> >>> perf-exec 1443864 [006] 2275506.209848: cbr: cbr: 41 freq: 4100 MHz (114%) 0 [unknown] ([unknown])
> >>> ls 1443864 [006] 2275506.209905: 1 branches:uH: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
> >>> ls 1443864 [006] 2275506.209908: 1 branches:uH: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
> >>>
> >>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> >>> ---
> >>> tools/perf/Documentation/perf-script.txt | 7 ++++---
> >>> tools/perf/builtin-script.c | 8 +++++++-
> >>> 2 files changed, 11 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> >>> index ff9a52e44688..fc79167c6bf8 100644
> >>> --- a/tools/perf/Documentation/perf-script.txt
> >>> +++ b/tools/perf/Documentation/perf-script.txt
> >>> @@ -132,9 +132,10 @@ OPTIONS
> >>> Comma separated list of fields to print. Options are:
> >>> comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
> >>> srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> >>> - brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> >>> - phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> >>> - machine_pid, vcpu, cgroup, retire_lat.
> >>> + brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> >>> + insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> >>> + code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> >>> +
> >>
> >> Further down, there are explanations for insn and insnlen. disasm
> >> could be added there.
> >>
> > Updated as:
> >
> > When doing instruction trace decoding, insn, disasm and insnlen give the
> > instruction bytes, disassembled instructions and the instruction length
> > of the current instruction respectively.
>
> I wondered about mentioning that disasm needs perf to be compiled with
> disassembler support, but with a permissive license it seems likely
> that libcapstone support would generally be built into perf, so that
> should be fine.
>
Yes, libcapstone has a permissive license. It's available on most Linux distros.
So, I updated as below:
When doing instruction trace decoding, insn, disasm and insnlen give the
instruction bytes, disassembled instructions (requires libcapstone support)
and the instruction length of the current instruction respectively.
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-22 10:46 ` Changbin Du
@ 2024-01-22 13:41 ` Andi Kleen
2024-01-22 14:05 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Andi Kleen @ 2024-01-22 13:41 UTC (permalink / raw)
To: Changbin Du
Cc: Adrian Hunter, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
> > >>
> > > Updated as:
> > >
> > > When doing instruction trace decoding, insn, disasm and insnlen give the
> > > instruction bytes, disassembled instructions and the instruction length
> > > of the current instruction respectively.
> >
> > I wondered about mentioning that disasm needs perf to be compiled with
> > disassembler support, but with a permissive license it seems likely
> > that libcapstone support would generally be built into perf, so that
> > should be fine.
> >
> Yes, libcapstone has a permissive license. It's available on most Linux distros.
What I've seen in the past is that people who build perf from source
often miss installing all the build the dependencies because perf continues
without an error. Make sure that if that happens and someone uses the option
there is a clear message that points to the build process.
-Andi
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions
2024-01-22 13:41 ` Andi Kleen
@ 2024-01-22 14:05 ` Changbin Du
0 siblings, 0 replies; 22+ messages in thread
From: Changbin Du @ 2024-01-22 14:05 UTC (permalink / raw)
To: Andi Kleen
Cc: Changbin Du, Adrian Hunter, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
linux-perf-users, Thomas Richter, changbin.du, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar
On Mon, Jan 22, 2024 at 05:41:42AM -0800, Andi Kleen wrote:
> > > >>
> > > > Updated as:
> > > >
> > > > When doing instruction trace decoding, insn, disasm and insnlen give the
> > > > instruction bytes, disassembled instructions and the instruction length
> > > > of the current instruction respectively.
> > >
> > > I wondered about mentioning that disasm needs perf to be compiled with
> > > disassembler support, but with a permissive license it seems likely
> > > that libcapstone support would generally be built into perf, so that
> > > should be fine.
> > >
> > Yes, libcapstone has a permissive license. It's available on most Linux distros.
>
> What I've seen in the past is that people who build perf from source
> often miss installing all the build the dependencies because perf continues
> without an error. Make sure that if that happens and someone uses the option
> there is a clear message that points to the build process.
>
Currently, perf build will show libcapstone available status at the beginning
of build and prompt user how to install it, as other dependencies do.
> -Andi
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option
2024-01-19 10:48 [PATCH v4 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
` (2 preceding siblings ...)
2024-01-19 10:48 ` [PATCH v4 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
@ 2024-01-19 10:48 ` Changbin Du
2024-01-19 18:39 ` Adrian Hunter
2024-01-19 10:48 ` [PATCH v4 5/5] perf: script: prefer capstone to XED Changbin Du
4 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-19 10:48 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Changbin Du
Now '--insn-trace' accept a argument to specify the output format:
- raw: display raw instructions.
- disasm: display mnemonic instructions (if capstone is installed).
$ sudo perf script --insn-trace=raw
ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: 48 89 e7
ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: e8 e8 0c 00 00
ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: f3 0f 1e fa
$ sudo perf script --insn-trace=disasm
ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: illegal instruction
ls 1443864 [006] 2275506.209908875: 7f216b426df4 _dl_start+0x4 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %rbp
ls 1443864 [006] 2275506.209908875: 7f216b426df5 _dl_start+0x5 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rbp
ls 1443864 [006] 2275506.209908875: 7f216b426df8 _dl_start+0x8 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %r15
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
tools/perf/Documentation/perf-script.txt | 6 +++---
tools/perf/builtin-script.c | 17 +++++++++++++----
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index fc79167c6bf8..9ae54f5bcb4d 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -442,9 +442,9 @@ include::itrace.txt[]
will be printed. Each entry has function name and file/line. Enabled by
default, disable with --no-inline.
---insn-trace::
- Show instruction stream for intel_pt traces. Combine with --xed to
- show disassembly.
+--insn-trace[=<raw|disasm>]::
+ Show raw or mnemonic instruction stream for intel_pt traces. You can
+ also combine raw instructions with --xed to show disassembly.
--xed::
Run xed disassembler on output. Requires installing the xed disassembler.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 12d886694f6c..2e3752b3b65a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3769,10 +3769,19 @@ static int perf_script__process_auxtrace_info(struct perf_session *session,
#endif
static int parse_insn_trace(const struct option *opt __maybe_unused,
- const char *str __maybe_unused,
- int unset __maybe_unused)
+ const char *str, int unset __maybe_unused)
{
- parse_output_fields(NULL, "+insn,-event,-period", 0);
+ const char *fields = "+insn,-event,-period";
+
+ if (str) {
+ if (strcmp(str, "disasm") == 0)
+ fields = "+disasm,-event,-period";
+ else if (strlen(str) != 0 && strcmp(str, "raw") != 0) {
+ fprintf(stderr, "Only accept raw|disasm\n");
+ return -EINVAL;
+ }
+ }
+ parse_output_fields(NULL, fields, 0);
itrace_parse_synth_opts(opt, "i0ns", 0);
symbol_conf.nanosecs = true;
return 0;
@@ -3918,7 +3927,7 @@ int cmd_script(int argc, const char **argv)
"only consider these symbols"),
OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
"Use with -S to list traced records within address range"),
- OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
+ OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, "raw|disasm",
"Decode instructions from itrace", parse_insn_trace),
OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
"Run xed disassembler on output", parse_xed),
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option
2024-01-19 10:48 ` [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
@ 2024-01-19 18:39 ` Adrian Hunter
2024-01-20 7:30 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-19 18:39 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo
On 19/01/24 12:48, Changbin Du wrote:
> Now '--insn-trace' accept a argument to specify the output format:
> - raw: display raw instructions.
> - disasm: display mnemonic instructions (if capstone is installed).
>
> $ sudo perf script --insn-trace=raw
> ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: 48 89 e7
> ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: e8 e8 0c 00 00
> ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: f3 0f 1e fa
>
> $ sudo perf script --insn-trace=disasm
> ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
> ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
> ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: illegal instruction
> ls 1443864 [006] 2275506.209908875: 7f216b426df4 _dl_start+0x4 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %rbp
> ls 1443864 [006] 2275506.209908875: 7f216b426df5 _dl_start+0x5 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rbp
> ls 1443864 [006] 2275506.209908875: 7f216b426df8 _dl_start+0x8 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %r15
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
> tools/perf/Documentation/perf-script.txt | 6 +++---
> tools/perf/builtin-script.c | 17 +++++++++++++----
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index fc79167c6bf8..9ae54f5bcb4d 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -442,9 +442,9 @@ include::itrace.txt[]
> will be printed. Each entry has function name and file/line. Enabled by
> default, disable with --no-inline.
>
> ---insn-trace::
> - Show instruction stream for intel_pt traces. Combine with --xed to
> - show disassembly.
> +--insn-trace[=<raw|disasm>]::
> + Show raw or mnemonic instruction stream for intel_pt traces. You can
> + also combine raw instructions with --xed to show disassembly.
Perhaps this is a bit clearer:
Show instruction stream in bytes (raw) or disassembled (disasm)
for intel_pt traces. The default is 'raw'. To use xed, combine
'raw' with --xed to show disassembly done by xed.
>
> --xed::
> Run xed disassembler on output. Requires installing the xed disassembler.
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 12d886694f6c..2e3752b3b65a 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3769,10 +3769,19 @@ static int perf_script__process_auxtrace_info(struct perf_session *session,
> #endif
>
> static int parse_insn_trace(const struct option *opt __maybe_unused,
> - const char *str __maybe_unused,
> - int unset __maybe_unused)
> + const char *str, int unset __maybe_unused)
> {
> - parse_output_fields(NULL, "+insn,-event,-period", 0);
> + const char *fields = "+insn,-event,-period";
> +
> + if (str) {
> + if (strcmp(str, "disasm") == 0)
> + fields = "+disasm,-event,-period";
> + else if (strlen(str) != 0 && strcmp(str, "raw") != 0) {
> + fprintf(stderr, "Only accept raw|disasm\n");
> + return -EINVAL;
> + }
> + }
> + parse_output_fields(NULL, fields, 0);
> itrace_parse_synth_opts(opt, "i0ns", 0);
> symbol_conf.nanosecs = true;
> return 0;
> @@ -3918,7 +3927,7 @@ int cmd_script(int argc, const char **argv)
> "only consider these symbols"),
> OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
> "Use with -S to list traced records within address range"),
> - OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
> + OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, "raw|disasm",
> "Decode instructions from itrace", parse_insn_trace),
> OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
> "Run xed disassembler on output", parse_xed),
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option
2024-01-19 18:39 ` Adrian Hunter
@ 2024-01-20 7:30 ` Changbin Du
0 siblings, 0 replies; 22+ messages in thread
From: Changbin Du @ 2024-01-20 7:30 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo
On Fri, Jan 19, 2024 at 08:39:50PM +0200, Adrian Hunter wrote:
> On 19/01/24 12:48, Changbin Du wrote:
> > Now '--insn-trace' accept a argument to specify the output format:
> > - raw: display raw instructions.
> > - disasm: display mnemonic instructions (if capstone is installed).
> >
> > $ sudo perf script --insn-trace=raw
> > ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: 48 89 e7
> > ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: e8 e8 0c 00 00
> > ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: f3 0f 1e fa
> >
> > $ sudo perf script --insn-trace=disasm
> > ls 1443864 [006] 2275506.209908875: 7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rdi
> > ls 1443864 [006] 2275506.209908875: 7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: callq _dl_start+0x0
> > ls 1443864 [006] 2275506.209908875: 7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: illegal instruction
> > ls 1443864 [006] 2275506.209908875: 7f216b426df4 _dl_start+0x4 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %rbp
> > ls 1443864 [006] 2275506.209908875: 7f216b426df5 _dl_start+0x5 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: movq %rsp, %rbp
> > ls 1443864 [006] 2275506.209908875: 7f216b426df8 _dl_start+0x8 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: pushq %r15
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> > tools/perf/Documentation/perf-script.txt | 6 +++---
> > tools/perf/builtin-script.c | 17 +++++++++++++----
> > 2 files changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index fc79167c6bf8..9ae54f5bcb4d 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -442,9 +442,9 @@ include::itrace.txt[]
> > will be printed. Each entry has function name and file/line. Enabled by
> > default, disable with --no-inline.
> >
> > ---insn-trace::
> > - Show instruction stream for intel_pt traces. Combine with --xed to
> > - show disassembly.
> > +--insn-trace[=<raw|disasm>]::
> > + Show raw or mnemonic instruction stream for intel_pt traces. You can
> > + also combine raw instructions with --xed to show disassembly.
>
> Perhaps this is a bit clearer:
>
> Show instruction stream in bytes (raw) or disassembled (disasm)
> for intel_pt traces. The default is 'raw'. To use xed, combine
> 'raw' with --xed to show disassembly done by xed.
>
Updated, thanks.
> >
> > --xed::
> > Run xed disassembler on output. Requires installing the xed disassembler.
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 12d886694f6c..2e3752b3b65a 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -3769,10 +3769,19 @@ static int perf_script__process_auxtrace_info(struct perf_session *session,
> > #endif
> >
> > static int parse_insn_trace(const struct option *opt __maybe_unused,
> > - const char *str __maybe_unused,
> > - int unset __maybe_unused)
> > + const char *str, int unset __maybe_unused)
> > {
> > - parse_output_fields(NULL, "+insn,-event,-period", 0);
> > + const char *fields = "+insn,-event,-period";
> > +
> > + if (str) {
> > + if (strcmp(str, "disasm") == 0)
> > + fields = "+disasm,-event,-period";
> > + else if (strlen(str) != 0 && strcmp(str, "raw") != 0) {
> > + fprintf(stderr, "Only accept raw|disasm\n");
> > + return -EINVAL;
> > + }
> > + }
> > + parse_output_fields(NULL, fields, 0);
> > itrace_parse_synth_opts(opt, "i0ns", 0);
> > symbol_conf.nanosecs = true;
> > return 0;
> > @@ -3918,7 +3927,7 @@ int cmd_script(int argc, const char **argv)
> > "only consider these symbols"),
> > OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
> > "Use with -S to list traced records within address range"),
> > - OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
> > + OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, "raw|disasm",
> > "Decode instructions from itrace", parse_insn_trace),
> > OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
> > "Run xed disassembler on output", parse_xed),
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 5/5] perf: script: prefer capstone to XED
2024-01-19 10:48 [PATCH v4 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
` (3 preceding siblings ...)
2024-01-19 10:48 ` [PATCH v4 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
@ 2024-01-19 10:48 ` Changbin Du
2024-01-19 18:40 ` Adrian Hunter
4 siblings, 1 reply; 22+ messages in thread
From: Changbin Du @ 2024-01-19 10:48 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Changbin Du
Now perf can show assembly instructions with libcapstone for x86, and the
capstone is better in general.
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
tools/perf/Documentation/perf-intel-pt.txt | 11 +++++------
tools/perf/ui/browsers/res_sample.c | 2 +-
tools/perf/ui/browsers/scripts.c | 2 +-
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
index 2109690b0d5f..8e62f23f7178 100644
--- a/tools/perf/Documentation/perf-intel-pt.txt
+++ b/tools/perf/Documentation/perf-intel-pt.txt
@@ -115,9 +115,8 @@ toggle respectively.
perf script also supports higher level ways to dump instruction traces:
- perf script --insn-trace --xed
+ perf script --insn-trace=disasm
-Dump all instructions. This requires installing the xed tool (see XED below)
Dumping all instructions in a long trace can be fairly slow. It is usually better
to start with higher level decoding, like
@@ -130,12 +129,12 @@ or
and then select a time range of interest. The time range can then be examined
in detail with
- perf script --time starttime,stoptime --insn-trace --xed
+ perf script --time starttime,stoptime --insn-trace=disasm
While examining the trace it's also useful to filter on specific CPUs using
the -C option
- perf script --time starttime,stoptime --insn-trace --xed -C 1
+ perf script --time starttime,stoptime --insn-trace=disasm -C 1
Dump all instructions in time range on CPU 1.
@@ -1306,7 +1305,7 @@ Without timestamps, --per-thread must be specified to distinguish threads.
perf script can be used to provide an instruction trace
- $ perf script --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
+ $ perf script --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
CPU 0/KVM 1440 ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
CPU 0/KVM 1440 ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
CPU 0/KVM 1440 ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
@@ -1407,7 +1406,7 @@ There were none.
'perf script' can be used to provide an instruction trace showing timestamps
- $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
+ $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
index 7cb2d6678039..1022baefaf45 100644
--- a/tools/perf/ui/browsers/res_sample.c
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -83,7 +83,7 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
r->tid ? "--tid " : "",
r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
extra_format,
- rstype == A_ASM ? "-F +insn --xed" :
+ rstype == A_ASM ? "-F +insn_disasm" :
rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
symbol_conf.inline_name ? "--inline" : "",
"--show-lost-events ",
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 47d2c7a8cbe1..3efc76c621c4 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -107,7 +107,7 @@ static int list_scripts(char *script_name, bool *custom,
if (evsel)
attr_to_script(scriptc.extra_format, &evsel->core.attr);
add_script_option("Show individual samples", "", &scriptc);
- add_script_option("Show individual samples with assembler", "-F +insn --xed",
+ add_script_option("Show individual samples with assembler", "-F +insn_disasm",
&scriptc);
add_script_option("Show individual samples with source", "-F +srcline,+srccode",
&scriptc);
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 5/5] perf: script: prefer capstone to XED
2024-01-19 10:48 ` [PATCH v4 5/5] perf: script: prefer capstone to XED Changbin Du
@ 2024-01-19 18:40 ` Adrian Hunter
2024-01-20 7:26 ` Changbin Du
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2024-01-19 18:40 UTC (permalink / raw)
To: Changbin Du
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
Thomas Richter, changbin.du, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo
On 19/01/24 12:48, Changbin Du wrote:
> Now perf can show assembly instructions with libcapstone for x86, and the
> capstone is better in general.
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
> tools/perf/Documentation/perf-intel-pt.txt | 11 +++++------
> tools/perf/ui/browsers/res_sample.c | 2 +-
> tools/perf/ui/browsers/scripts.c | 2 +-
> 3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
> index 2109690b0d5f..8e62f23f7178 100644
> --- a/tools/perf/Documentation/perf-intel-pt.txt
> +++ b/tools/perf/Documentation/perf-intel-pt.txt
> @@ -115,9 +115,8 @@ toggle respectively.
>
> perf script also supports higher level ways to dump instruction traces:
>
> - perf script --insn-trace --xed
> + perf script --insn-trace=disasm
Please add also:
or to use the xed disassembler, which requires installing the xed tool
(see XED below):
perf script --insn-trace --xed
>
> -Dump all instructions. This requires installing the xed tool (see XED below)
> Dumping all instructions in a long trace can be fairly slow. It is usually better
> to start with higher level decoding, like
>
> @@ -130,12 +129,12 @@ or
> and then select a time range of interest. The time range can then be examined
> in detail with
>
> - perf script --time starttime,stoptime --insn-trace --xed
> + perf script --time starttime,stoptime --insn-trace=disasm
>
> While examining the trace it's also useful to filter on specific CPUs using
> the -C option
>
> - perf script --time starttime,stoptime --insn-trace --xed -C 1
> + perf script --time starttime,stoptime --insn-trace=disasm -C 1
>
> Dump all instructions in time range on CPU 1.
>
> @@ -1306,7 +1305,7 @@ Without timestamps, --per-thread must be specified to distinguish threads.
>
> perf script can be used to provide an instruction trace
>
> - $ perf script --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
> + $ perf script --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
> CPU 0/KVM 1440 ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
> CPU 0/KVM 1440 ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
> CPU 0/KVM 1440 ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
> @@ -1407,7 +1406,7 @@ There were none.
>
> 'perf script' can be used to provide an instruction trace showing timestamps
>
> - $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
> + $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
> CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
> CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
> CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
> diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
> index 7cb2d6678039..1022baefaf45 100644
> --- a/tools/perf/ui/browsers/res_sample.c
> +++ b/tools/perf/ui/browsers/res_sample.c
> @@ -83,7 +83,7 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
> r->tid ? "--tid " : "",
> r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
> extra_format,
> - rstype == A_ASM ? "-F +insn --xed" :
> + rstype == A_ASM ? "-F +insn_disasm" :
insn_disasm -> disasm
> rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
> symbol_conf.inline_name ? "--inline" : "",
> "--show-lost-events ",
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 47d2c7a8cbe1..3efc76c621c4 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -107,7 +107,7 @@ static int list_scripts(char *script_name, bool *custom,
> if (evsel)
> attr_to_script(scriptc.extra_format, &evsel->core.attr);
> add_script_option("Show individual samples", "", &scriptc);
> - add_script_option("Show individual samples with assembler", "-F +insn --xed",
> + add_script_option("Show individual samples with assembler", "-F +insn_disasm",
insn_disasm -> disasm
> &scriptc);
> add_script_option("Show individual samples with source", "-F +srcline,+srccode",
> &scriptc);
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 5/5] perf: script: prefer capstone to XED
2024-01-19 18:40 ` Adrian Hunter
@ 2024-01-20 7:26 ` Changbin Du
0 siblings, 0 replies; 22+ messages in thread
From: Changbin Du @ 2024-01-20 7:26 UTC (permalink / raw)
To: Adrian Hunter
Cc: Changbin Du, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users,
Andi Kleen, Thomas Richter, changbin.du, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo
On Fri, Jan 19, 2024 at 08:40:20PM +0200, Adrian Hunter wrote:
> On 19/01/24 12:48, Changbin Du wrote:
> > Now perf can show assembly instructions with libcapstone for x86, and the
> > capstone is better in general.
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> > tools/perf/Documentation/perf-intel-pt.txt | 11 +++++------
> > tools/perf/ui/browsers/res_sample.c | 2 +-
> > tools/perf/ui/browsers/scripts.c | 2 +-
> > 3 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
> > index 2109690b0d5f..8e62f23f7178 100644
> > --- a/tools/perf/Documentation/perf-intel-pt.txt
> > +++ b/tools/perf/Documentation/perf-intel-pt.txt
> > @@ -115,9 +115,8 @@ toggle respectively.
> >
> > perf script also supports higher level ways to dump instruction traces:
> >
> > - perf script --insn-trace --xed
> > + perf script --insn-trace=disasm
>
> Please add also:
>
> or to use the xed disassembler, which requires installing the xed tool
> (see XED below):
>
> perf script --insn-trace --xed
>
Added, thanks.
> >
> > -Dump all instructions. This requires installing the xed tool (see XED below)
> > Dumping all instructions in a long trace can be fairly slow. It is usually better
> > to start with higher level decoding, like
> >
> > @@ -130,12 +129,12 @@ or
> > and then select a time range of interest. The time range can then be examined
> > in detail with
> >
> > - perf script --time starttime,stoptime --insn-trace --xed
> > + perf script --time starttime,stoptime --insn-trace=disasm
> >
> > While examining the trace it's also useful to filter on specific CPUs using
> > the -C option
> >
> > - perf script --time starttime,stoptime --insn-trace --xed -C 1
> > + perf script --time starttime,stoptime --insn-trace=disasm -C 1
> >
> > Dump all instructions in time range on CPU 1.
> >
> > @@ -1306,7 +1305,7 @@ Without timestamps, --per-thread must be specified to distinguish threads.
> >
> > perf script can be used to provide an instruction trace
> >
> > - $ perf script --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
> > + $ perf script --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
> > CPU 0/KVM 1440 ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
> > CPU 0/KVM 1440 ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
> > CPU 0/KVM 1440 ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
> > @@ -1407,7 +1406,7 @@ There were none.
> >
> > 'perf script' can be used to provide an instruction trace showing timestamps
> >
> > - $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
> > + $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
> > CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms]) movq 0x48(%rax), %r9
> > CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms]) movq 0x50(%rax), %r10
> > CPU 1/KVM 17006 [001] 11500.262865593: ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms]) movq 0x58(%rax), %r11
> > diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
> > index 7cb2d6678039..1022baefaf45 100644
> > --- a/tools/perf/ui/browsers/res_sample.c
> > +++ b/tools/perf/ui/browsers/res_sample.c
> > @@ -83,7 +83,7 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
> > r->tid ? "--tid " : "",
> > r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
> > extra_format,
> > - rstype == A_ASM ? "-F +insn --xed" :
> > + rstype == A_ASM ? "-F +insn_disasm" :
>
> insn_disasm -> disasm
>
Fixed. I forgot to commit this change for last version.
> > rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
> > symbol_conf.inline_name ? "--inline" : "",
> > "--show-lost-events ",
> > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> > index 47d2c7a8cbe1..3efc76c621c4 100644
> > --- a/tools/perf/ui/browsers/scripts.c
> > +++ b/tools/perf/ui/browsers/scripts.c
> > @@ -107,7 +107,7 @@ static int list_scripts(char *script_name, bool *custom,
> > if (evsel)
> > attr_to_script(scriptc.extra_format, &evsel->core.attr);
> > add_script_option("Show individual samples", "", &scriptc);
> > - add_script_option("Show individual samples with assembler", "-F +insn --xed",
> > + add_script_option("Show individual samples with assembler", "-F +insn_disasm",
>
> insn_disasm -> disasm
>
Fixed.
> > &scriptc);
> > add_script_option("Show individual samples with source", "-F +srcline,+srccode",
> > &scriptc);
>
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 22+ messages in thread