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

From: Arnaldo Carvalho de Melo
Date: Thu May 02 2024 - 09:41:28 EST


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