Re: [PATCH v4 5/5] perf: script: prefer capstone to XED

[Changbin Du]

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@xxxxxxxxxx>
> > ---
> >  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