Received-SPF: pass (google.com: domain of linux-kernel+bounces-28456-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 17 Jan 2024 10:17:23 +0800
From: Changbin Du <changbin.du@huawei.com>
To: Thomas Richter <tmricht@linux.ibm.com>
CC: Changbin Du <changbin.du@huawei.com>, Peter Zijlstra
	<peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de
 Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander
 Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian
 Hunter <adrian.hunter@intel.com>, <linux-kernel@vger.kernel.org>,
	<linux-perf-users@vger.kernel.org>
Subject: Re: [PATCH 2/3] perf: script: use capstone disasm engine to show
 assembly instructions
Message-ID: <20240117021723.bhxvgha74syz5fbg@M910t>
References: <20240116113437.1507537-1-changbin.du@huawei.com>
 <20240116113437.1507537-3-changbin.du@huawei.com>
 <f81cf759-d5a4-4b46-bd2c-2e11355447a8@linux.ibm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f81cf759-d5a4-4b46-bd2c-2e11355447a8@linux.ibm.com>

On Tue, Jan 16, 2024 at 03:26:52PM +0100, Thomas Richter wrote:
> On 1/16/24 12:34, 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.
> > 
> > Before:
> > $ sudo perf record --event intel_pt//u -- ls
> > $ sudo perf script --insn-trace
> >             perf 17423 [000] 423271.557970005:      7f2d95f16217 __GI___ioctl+0x7 (/lib/x86_64-linux-gnu/libc-2.27.so) insn: 48 3d 01 f0 ff ff
> >             perf 17423 [000] 423271.557970005:      7f2d95f1621d __GI___ioctl+0xd (/lib/x86_64-linux-gnu/libc-2.27.so) insn: 73 01
> >             perf 17423 [000] 423271.557970338:      7f2d95f1621f __GI___ioctl+0xf (/lib/x86_64-linux-gnu/libc-2.27.so) insn: c3
> >             perf 17423 [000] 423271.557970338:      5593ad3346d7 perf_evsel__enable_cpu+0x97 (/work/linux/tools/perf/perf) insn: 85 c0
> >             perf 17423 [000] 423271.557970338:      5593ad3346d9 perf_evsel__enable_cpu+0x99 (/work/linux/tools/perf/perf) insn: 75 12
> >             perf 17423 [000] 423271.557970338:      5593ad3346db perf_evsel__enable_cpu+0x9b (/work/linux/tools/perf/perf) insn: 49 8b 84 24 a8 00 00 00
> >             perf 17423 [000] 423271.557970338:      5593ad3346e3 perf_evsel__enable_cpu+0xa3 (/work/linux/tools/perf/perf) insn: 48 8b 50 20
> > 
> > After:
> > $ sudo perf script --insn-trace
> >             perf 17423 [000] 423271.557970005:      7f2d95f16217 __GI___ioctl+0x7 (/lib/x86_64-linux-gnu/libc-2.27.so) insn: cmpq $-0xfff, %rax
> >             perf 17423 [000] 423271.557970005:      7f2d95f1621d __GI___ioctl+0xd (/lib/x86_64-linux-gnu/libc-2.27.so) insn: jae __GI___ioctl+0x10
> >             perf 17423 [000] 423271.557970338:      7f2d95f1621f __GI___ioctl+0xf (/lib/x86_64-linux-gnu/libc-2.27.so) insn: retq
> >             perf 17423 [000] 423271.557970338:      5593ad3346d7 perf_evsel__enable_cpu+0x97 (/work/linux/tools/perf/perf) insn: testl %eax, %eax
> >             perf 17423 [000] 423271.557970338:      5593ad3346d9 perf_evsel__enable_cpu+0x99 (/work/linux/tools/perf/perf) insn: jne perf_evsel__enable_cpu+0xad
> >             perf 17423 [000] 423271.557970338:      5593ad3346db perf_evsel__enable_cpu+0x9b (/work/linux/tools/perf/perf) insn: movq 0xa8(%r12), %rax
> >             perf 17423 [000] 423271.557970338:      5593ad3346e3 perf_evsel__enable_cpu+0xa3 (/work/linux/tools/perf/perf) insn: movq 0x20(%rax), %rdx
> >             perf 17423 [000] 423271.557970338:      5593ad3346e7 perf_evsel__enable_cpu+0xa7 (/work/linux/tools/perf/perf) insn: cmpl %edx, %ebx
> >             perf 17423 [000] 423271.557970338:      5593ad3346e9 perf_evsel__enable_cpu+0xa9 (/work/linux/tools/perf/perf) insn: jl perf_evsel__enable_cpu+0x60
> >             perf 17423 [000] 423271.557970338:      5593ad3346eb perf_evsel__enable_cpu+0xab (/work/linux/tools/perf/perf) insn: xorl %eax, %eax
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> 
> ....
> 
> > diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> > new file mode 100644
> > index 000000000000..c8d9741748cd
> > --- /dev/null
> > +++ b/tools/perf/util/print_insn.c
> > @@ -0,0 +1,118 @@
> > +// 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 {
> > +		return -1;
> > +	}
> > ...
> 
> Did you forgot to support s390? Or was it omitted on intention?
> Something along the lines will support s390:
> 
>  # git diff util/print_insn.c
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index c8d9741748cd..c5127910c75b 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -41,6 +41,9 @@ static int capstone_init(struct machine *machine, csh *cs_handle)
>         } else if (machine__normalized_is(machine, "arm")) {
>                 arch = CS_ARCH_ARM;
>                 mode = CS_MODE_ARM + CS_MODE_V8;
> +       } else if (machine__normalized_is(machine, "s390x")) {
> +               arch = CS_ARCH_SYSZ;
> +               mode = CS_MODE_BIG_ENDIAN;
>         } else {
>                 return -1;
>         }
>   #
> 
> Thanks a lot for including these line in your next version.
> 
No problem, thanks.

> -- 
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> IBM Deutschland Research & Development GmbH
> 
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
> 
> Geschäftsführung: David Faller
> 
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
> 

-- 
Cheers,
Changbin Du