Re: [PATCH 02/10] x86/insn: Fix PUSH instruction in x86 instruction decoder opcode map

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, May 02, 2024 at 01:58:45PM +0300, Adrian Hunter wrote:
> The x86 instruction decoder is used not only for decoding kernel
> instructions. It is also used by perf uprobes (user space probes) and by
> perf tools Intel Processor Trace decoding. Consequently, it needs to
> support instructions executed by user space also.

I wonder if we should do it this way, i.e. updating both tools/ and
kernel source code in the same patch.

I think the best is to update the kernel bits, then, after that is
merged, do the tooling part.

To avoid possible, yet unlikely, clashes in linux-next, for instance.

- Arnaldo
 
> Opcode 0x68 PUSH instruction is currently defined as 64-bit operand size
> only i.e. (d64). That was based on Intel SDM Opcode Map. However that is
> contradicted by the Instruction Set Reference section for PUSH in the
> same manual.
> 
> Remove 64-bit operand size only annotation from opcode 0x68 PUSH
> instruction.
> 
> Example:
> 
>   $ cat pushw.s
>   .global  _start
>   .text
>   _start:
>           pushw   $0x1234
>           mov     $0x1,%eax   # system call number (sys_exit)
>           int     $0x80
>   $ as -o pushw.o pushw.s
>   $ ld -s -o pushw pushw.o
>   $ objdump -d pushw | tail -4
>   0000000000401000 <.text>:
>     401000:       66 68 34 12             pushw  $0x1234
>     401004:       b8 01 00 00 00          mov    $0x1,%eax
>     401009:       cd 80                   int    $0x80
>   $ perf record -e intel_pt//u ./pushw
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.014 MB perf.data ]
> 
>  Before:
> 
>   $ perf script --insn-trace=disasm
>   Warning:
>   1 instruction trace errors
>            pushw   10349 [000] 10586.869237014:            401000 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           pushw $0x1234
>            pushw   10349 [000] 10586.869237014:            401006 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb %al, (%rax)
>            pushw   10349 [000] 10586.869237014:            401008 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb %cl, %ch
>            pushw   10349 [000] 10586.869237014:            40100a [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb $0x2e, (%rax)
>    instruction trace error type 1 time 10586.869237224 cpu 0 pid 10349 tid 10349 ip 0x40100d code 6: Trace doesn't match instruction
> 
>  After:
> 
>   $ perf script --insn-trace=disasm
>              pushw   10349 [000] 10586.869237014:            401000 [unknown] (./pushw)           pushw $0x1234
>              pushw   10349 [000] 10586.869237014:            401004 [unknown] (./pushw)           movl $1, %eax
> 
> Fixes: eb13296cfaf6 ("x86: Instruction decoder API")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  arch/x86/lib/x86-opcode-map.txt       | 2 +-
>  tools/arch/x86/lib/x86-opcode-map.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
> index c94988d5130d..4ea2e6adb477 100644
> --- a/arch/x86/lib/x86-opcode-map.txt
> +++ b/arch/x86/lib/x86-opcode-map.txt
> @@ -148,7 +148,7 @@ AVXcode:
>  65: SEG=GS (Prefix)
>  66: Operand-Size (Prefix)
>  67: Address-Size (Prefix)
> -68: PUSH Iz (d64)
> +68: PUSH Iz
>  69: IMUL Gv,Ev,Iz
>  6a: PUSH Ib (d64)
>  6b: IMUL Gv,Ev,Ib
> diff --git a/tools/arch/x86/lib/x86-opcode-map.txt b/tools/arch/x86/lib/x86-opcode-map.txt
> index c94988d5130d..4ea2e6adb477 100644
> --- a/tools/arch/x86/lib/x86-opcode-map.txt
> +++ b/tools/arch/x86/lib/x86-opcode-map.txt
> @@ -148,7 +148,7 @@ AVXcode:
>  65: SEG=GS (Prefix)
>  66: Operand-Size (Prefix)
>  67: Address-Size (Prefix)
> -68: PUSH Iz (d64)
> +68: PUSH Iz
>  69: IMUL Gv,Ev,Iz
>  6a: PUSH Ib (d64)
>  6b: IMUL Gv,Ev,Ib
> -- 
> 2.34.1