Re: [PATCH 3/3] objtool/x86: Fix NOP decode

[Alexandre Chartre <alexandre.chartre@oracle.com>]

On 9/24/25 20:41, Peter Zijlstra wrote:
> On Wed, Sep 24, 2025 at 07:34:00PM +0200, Alexandre Chartre wrote:
>>
>> On 9/24/25 15:45, Peter Zijlstra wrote:
>>> For x86_64 the kernel consistently uses 2 instructions for all NOPs:
>>>
>>>     90       - NOP
>>>     0f 1f /0 - NOPL
>>>
>>>
>>> Notably:
>>>
>>>    - REP NOP is PAUSE, not a NOP instruction.
>>>
>>>    - 0f {0c...0f} is reserved space,
>>>      except for 0f 0d /1, which is PREFETCHW, not a NOP.
>>>
>>>    - 0f {19,1c...1f} is reserved space,
>>>      except for 0f 1f /0, which is NOPL.
>>>
>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>> ---
>>>    tools/objtool/arch/x86/decode.c |   12 +++++++-----
>>>    1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> --- a/tools/objtool/arch/x86/decode.c
>>> +++ b/tools/objtool/arch/x86/decode.c
>>> @@ -494,7 +494,8 @@ int arch_decode_instruction(struct objto
>>>    		break;
>>>    	case 0x90:
>>> +		if (prefix != 0xf3) /* REP NOP := PAUSE */
>>> +			insn->type = INSN_NOP;
>>>    		break;
>>
>> So this covers NOP1 (0x90) and NOP2 (0x66 0x90), right?
> 
> Yes. Everything with opcode 0x90, except 0xf3 0x90, which as stated is
> PAUSE.
> 

What about 0x49 0x90, which is xchg (XCHG r8,rAX) ?


>>>    	case 0x9c:
>>> @@ -547,13 +548,14 @@ int arch_decode_instruction(struct objto
>>>    		} else if (op2 == 0x0b || op2 == 0xb9) {
>>> +			/* ud2, ud1 */
>>>    			insn->type = INSN_BUG;
>>> +		} else if (op2 == 0x1f) {
>>> +			/* 0f 1f /0 := NOPL */
>>> +			if (modrm_reg == 0)
>>> +				insn->type = INSN_NOP;
>>>    		} else if (op2 == 0x1e) {
>>
>> And this covers all other NOPs (0x0f 0x1f ...), including NOP6 which has
>> a 0x66 preifx (0x66 0xf 0x1f ...) ?
> 
> Sorta, it accepts everything with opcode 0f 1f and modrm_reg==0, which is
> how NOPL is encoded.
> 
> Both: 66 66 66 66 66 66 66 66 66 66 66 66 66 66 90 (NOP15)
> And:  66 66 66 66 66 66 66 0f 1f 84 00 00 00 00 00 (NOP15)
> 
> will be accepted here as max length instructions. The kernel will not
> actually use those, since a bunch of micro archs have decode penalties
> for too many prefixes.
> 
>>  From arch/x86/include/asm/nops.h we have:
> 
> You're looking at old code :-)
> 

Correct, I was on the 5.15 branch.

alex.

>> /*
>>   * Generic 64bit nops from GAS:
>>   *
>>   * 1: nop
>>   * 2: osp nop
>>   * 3: nopl (%eax)
>>   * 4: nopl 0x00(%eax)
>>   * 5: nopl 0x00(%eax,%eax,1)
>>   * 6: osp nopl 0x00(%eax,%eax,1)
>>   * 7: nopl 0x00000000(%eax)
>>   * 8: nopl 0x00000000(%eax,%eax,1)
> 
>   * 9: cs nopl 0x00000000(%eax,%eax,1)
>   * 10: osp cs nopl 0x00000000(%eax,%eax,1)
>   * 11: osp osp cs nopl 0x00000000(%eax,%eax,1)
> 
>>   */
>> #define BYTES_NOP1      0x90
>> #define BYTES_NOP2      0x66,BYTES_NOP1
>> #define BYTES_NOP3      0x0f,0x1f,0x00
>> #define BYTES_NOP4      0x0f,0x1f,0x40,0x00
>> #define BYTES_NOP5      0x0f,0x1f,0x44,0x00,0x00
>> #define BYTES_NOP6      0x66,BYTES_NOP5
>> #define BYTES_NOP7      0x0f,0x1f,0x80,0x00,0x00,0x00,0x00
>> #define BYTES_NOP8      0x0f,0x1f,0x84,0x00,0x00,0x00,0x00,0x00
> 
> #define BYTES_NOP9      0x2e,BYTES_NOP8
> #define BYTES_NOP10     0x66,BYTES_NOP9
> #define BYTES_NOP11     0x66,BYTES_NOP10
> 
> But yes, first two are NOP and then it switches to NOPL for 3 bytes and
> longer (2 opcode, 1 modrm). Where for 11 bytes we have:
> 
>   - 3 prefixes
>   - 2 opcode
>   - 1 modrm
>   - 1 sib
>   - 4 displacement
>