[PATCH 0/3] objtool: Few x86 decoder updates

[PATCH 0/3] objtool: Few x86 decoder updates

[Peter Zijlstra]

Hi,

few patches that update the 0xea/udb situation and fix up the x86_64 NOP
decoding.

[PATCH 1/3] objtool/x86: Remove 0xea hack

[Peter Zijlstra]

Was properly fixed in the decoder with commit 4b626015e1bf ("x86/insn:
Stop decoding i64 instructions in x86-64 mode at opcode")

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/arch/x86/decode.c |    9 ---------
 1 file changed, 9 deletions(-)

--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -189,15 +189,6 @@ int arch_decode_instruction(struct objto
 	op2 = ins.opcode.bytes[1];
 	op3 = ins.opcode.bytes[2];
 
-	/*
-	 * XXX hack, decoder is buggered and thinks 0xea is 7 bytes long.
-	 */
-	if (op1 == 0xea) {
-		insn->len = 1;
-		insn->type = INSN_BUG;
-		return 0;
-	}
-
 	if (ins.rex_prefix.nbytes) {
 		rex = ins.rex_prefix.bytes[0];
 		rex_w = X86_REX_W(rex) >> 3;

Re: [PATCH 1/3] objtool/x86: Remove 0xea hack

[Alexandre Chartre]

On 9/24/25 15:45, Peter Zijlstra wrote:
> Was properly fixed in the decoder with commit 4b626015e1bf ("x86/insn:
> Stop decoding i64 instructions in x86-64 mode at opcode")
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>   tools/objtool/arch/x86/decode.c |    9 ---------
>   1 file changed, 9 deletions(-)
> 
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -189,15 +189,6 @@ int arch_decode_instruction(struct objto
>   	op2 = ins.opcode.bytes[1];
>   	op3 = ins.opcode.bytes[2];
>   
> -	/*
> -	 * XXX hack, decoder is buggered and thinks 0xea is 7 bytes long.
> -	 */
> -	if (op1 == 0xea) {
> -		insn->len = 1;
> -		insn->type = INSN_BUG;
> -		return 0;
> -	}
> -
>   	if (ins.rex_prefix.nbytes) {
>   		rex = ins.rex_prefix.bytes[0];
>   		rex_w = X86_REX_W(rex) >> 3;
> 
> 

Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

alex.

[tip: objtool/core] objtool/x86: Remove 0xea hack

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     c5df4e1ab8c00c4dc13094fa44e219bc48d910f4
Gitweb:        https://git.kernel.org/tip/c5df4e1ab8c00c4dc13094fa44e219bc48d910f4
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 24 Sep 2025 15:22:46 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 14 Oct 2025 13:43:10 +02:00

objtool/x86: Remove 0xea hack

Was properly fixed in the decoder with commit 4b626015e1bf ("x86/insn:
Stop decoding i64 instructions in x86-64 mode at opcode")

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/arch/x86/decode.c |  9 ---------
 1 file changed, 9 deletions(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0ad5cc7..ce16fb2 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -189,15 +189,6 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 	op2 = ins.opcode.bytes[1];
 	op3 = ins.opcode.bytes[2];
 
-	/*
-	 * XXX hack, decoder is buggered and thinks 0xea is 7 bytes long.
-	 */
-	if (op1 == 0xea) {
-		insn->len = 1;
-		insn->type = INSN_BUG;
-		return 0;
-	}
-
 	if (ins.rex_prefix.nbytes) {
 		rex = ins.rex_prefix.bytes[0];
 		rex_w = X86_REX_W(rex) >> 3;

[PATCH 2/3] objtool/x86: Add UDB support

[Peter Zijlstra]

Per commit 85a2d4a890dc ("x86,ibt: Use UDB instead of 0xEA"), make
sure objtool also recognises UDB as a #UD instruction.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/arch/x86/decode.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -683,6 +683,10 @@ int arch_decode_instruction(struct objto
 		insn->type = INSN_SYSRET;
 		break;
 
+	case 0xd6: /* udb */
+		insn->type = INSN_BUG;
+		break;
+
 	case 0xe0: /* loopne */
 	case 0xe1: /* loope */
 	case 0xe2: /* loop */

Re: [PATCH 2/3] objtool/x86: Add UDB support

[Alexandre Chartre]

On 9/24/25 15:45, Peter Zijlstra wrote:
> Per commit 85a2d4a890dc ("x86,ibt: Use UDB instead of 0xEA"), make
> sure objtool also recognises UDB as a #UD instruction.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>   tools/objtool/arch/x86/decode.c |    4 ++++
>   1 file changed, 4 insertions(+)
> 
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -683,6 +683,10 @@ int arch_decode_instruction(struct objto
>   		insn->type = INSN_SYSRET;
>   		break;
>   
> +	case 0xd6: /* udb */
> +		insn->type = INSN_BUG;
> +		break;
> +
>   	case 0xe0: /* loopne */
>   	case 0xe1: /* loope */
>   	case 0xe2: /* loop */
> 
> 

Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

alex.

[tip: objtool/core] objtool/x86: Add UDB support

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     76e1851a1bc28e760d6acc7a54ec9dce05717028
Gitweb:        https://git.kernel.org/tip/76e1851a1bc28e760d6acc7a54ec9dce05717028
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 24 Sep 2025 15:25:27 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 14 Oct 2025 13:43:11 +02:00

objtool/x86: Add UDB support

Per commit 85a2d4a890dc ("x86,ibt: Use UDB instead of 0xEA"), make
sure objtool also recognises UDB as a #UD instruction.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/arch/x86/decode.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index ce16fb2..ef6e96d 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -683,6 +683,10 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 		insn->type = INSN_SYSRET;
 		break;
 
+	case 0xd6: /* udb */
+		insn->type = INSN_BUG;
+		break;
+
 	case 0xe0: /* loopne */
 	case 0xe1: /* loope */
 	case 0xe2: /* loop */

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

[Peter Zijlstra]

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:
-		insn->type = INSN_NOP;
+		if (prefix != 0xf3) /* REP NOP := PAUSE */
+			insn->type = INSN_NOP;
 		break;
 
 	case 0x9c:
@@ -547,13 +548,14 @@ int arch_decode_instruction(struct objto
 
 		} else if (op2 == 0x0b || op2 == 0xb9) {
 
-			/* ud2 */
+			/* ud2, ud1 */
 			insn->type = INSN_BUG;
 
-		} else if (op2 == 0x0d || op2 == 0x1f) {
+		} else if (op2 == 0x1f) {
 
-			/* nopl/nopw */
-			insn->type = INSN_NOP;
+			/* 0f 1f /0 := NOPL */
+			if (modrm_reg == 0)
+				insn->type = INSN_NOP;
 
 		} else if (op2 == 0x1e) {
 

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

[Alexandre Chartre]

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:
> -		insn->type = INSN_NOP;
> +		if (prefix != 0xf3) /* REP NOP := PAUSE */
> +			insn->type = INSN_NOP;
>   		break;

So this covers NOP1 (0x90) and NOP2 (0x66 0x90), right?

>   
>   	case 0x9c:
> @@ -547,13 +548,14 @@ int arch_decode_instruction(struct objto
>   
>   		} else if (op2 == 0x0b || op2 == 0xb9) {
>   
> -			/* ud2 */
> +			/* ud2, ud1 */
>   			insn->type = INSN_BUG;
>   
> -		} else if (op2 == 0x0d || op2 == 0x1f) {
> +		} else if (op2 == 0x1f) {
>   
> -			/* nopl/nopw */
> -			insn->type = INSN_NOP;
> +			/* 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 ...) ?

 From arch/x86/include/asm/nops.h we have:

/*
  * 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)
  */
#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


alex.

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

[Peter Zijlstra]

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.

> >   	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 :-)

> /*
>  * 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

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

[Alexandre Chartre]

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
> 

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

[Peter Zijlstra]

On Thu, Sep 25, 2025 at 11:55:23AM +0200, Alexandre Chartre wrote:
> 
> 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) ?

Ooh, that is a nice one. Yes, that needs fixing. Thanks!

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

[Peter Zijlstra]

On Thu, Sep 25, 2025 at 12:03:23PM +0200, Peter Zijlstra wrote:

> > > > >    	case 0x90:
> > > > > +		if (prefix != 0xf3) /* REP NOP := PAUSE */
> > > > > +			insn->type = INSN_NOP;
> > > > >    		break;

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

I've made that:

	case 0x90:                                                                                                                                                                                                                        
		if (rex_b) /* XCHG %r8, %rax */ 
			break; 

		if (prefix == 0xf3) /* REP NOP := PAUSE */ 
			break; 

		insn->type = INSN_NOP; 
		break;

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

[Andrew Cooper]

> I've made that:
>
> 	case 0x90:                                                                                                                                                                                                                        
> 		if (rex_b) /* XCHG %r8, %rax */ 
> 			break; 
>
> 		if (prefix == 0xf3) /* REP NOP := PAUSE */ 
> 			break; 
>
> 		insn->type = INSN_NOP; 
> 		break;

Legacy prefixes can come in any order.  What is F3 66 90 ?

Also, VEX/EVEX/REX2 want excluding too, all of which can encode rex_b
differently.

Is it really only rex_b which prevents NOP becoming a pause, or is it
any REX prefix?  I would have thought it was any REX prefix.

~Andrew

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

[Peter Zijlstra]

On Thu, Sep 25, 2025 at 12:29:18PM +0100, Andrew Cooper wrote:
> > I've made that:
> >
> > 	case 0x90:                                                                                                                                                                                                                        
> > 		if (rex_b) /* XCHG %r8, %rax */ 
> > 			break; 
> >
> > 		if (prefix == 0xf3) /* REP NOP := PAUSE */ 
> > 			break; 
> >
> > 		insn->type = INSN_NOP; 
> > 		break;
> 
> Legacy prefixes can come in any order.  What is F3 66 90 ?
> 
> Also, VEX/EVEX/REX2 want excluding too, all of which can encode rex_b
> differently.

So luckily objtool only really cares about instructions as found in the
kernel text. Neither f3 66 90 nor VEX/EVEX/REX2 prefixes are of much
concern.

But yes.. I happen to have an insn_is_nop() function that can be used on
userspace, and that certainly wants to be taught about these... x86 is
such a pain :/

> Is it really only rex_b which prevents NOP becoming a pause, or is it
> any REX prefix?  I would have thought it was any REX prefix.

SDM opcode table and instruction reference seems consistent with f3
only.

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

[Alexandre Chartre]

On 9/25/25 14:43, Peter Zijlstra wrote:
> On Thu, Sep 25, 2025 at 12:29:18PM +0100, Andrew Cooper wrote:
>>> I've made that:
>>>
>>> 	case 0x90:
>>> 		if (rex_b) /* XCHG %r8, %rax */
>>> 			break;
>>>
>>> 		if (prefix == 0xf3) /* REP NOP := PAUSE */
>>> 			break;
>>>
>>> 		insn->type = INSN_NOP;
>>> 		break;
>>
>> Legacy prefixes can come in any order.  What is F3 66 90 ?
>>
>> Also, VEX/EVEX/REX2 want excluding too, all of which can encode rex_b
>> differently.
> 
> So luckily objtool only really cares about instructions as found in the
> kernel text. Neither f3 66 90 nor VEX/EVEX/REX2 prefixes are of much
> concern.

And it looks like objtool ignores VEX instructions earlier in the same function:

int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
                             unsigned long offset, unsigned int maxlen,
                             struct instruction *insn)
{
        ...

        if (ins.vex_prefix.nbytes)
                 return 0;

        ...
}

vex_prefix is set for VEX/EVEX/VEX3/VEX2.

alex.

> But yes.. I happen to have an insn_is_nop() function that can be used on
> userspace, and that certainly wants to be taught about these... x86 is
> such a pain :/
> 
>> Is it really only rex_b which prevents NOP becoming a pause, or is it
>> any REX prefix?  I would have thought it was any REX prefix.
> 
> SDM opcode table and instruction reference seems consistent with f3
> only.

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

[Peter Zijlstra]

On Thu, Sep 25, 2025 at 02:43:32PM +0200, Peter Zijlstra wrote:

> But yes.. I happen to have an insn_is_nop() function that can be used on
> userspace, and that certainly wants to be taught about these... x86 is
> such a pain :/

This is the delta I ended up with for my insn_is_nop() function to
support *VEX*/REX2.

I'll post the whole thing later.

--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -138,6 +138,10 @@ struct insn {
 #define X86_VEX_V(vex)	(((vex) & 0x78) >> 3)	/* VEX3 Byte2, VEX2 Byte1 */
 #define X86_VEX_P(vex)	((vex) & 0x03)		/* VEX3 Byte2, VEX2 Byte1 */
 #define X86_VEX_M_MAX	0x1f			/* VEX3.M Maximum value */
+/* EVEX bit fields */
+#define X86_EVEX_R4(vex) ((vex) & 0x10)		/* EVEX Byte1 */
+#define X86_EVEX_B4(vex) ((vex) & 0x08)		/* EVEX Byte1 */
+#define X86_EVEX_X4(vex) ((vex) & 0x04)		/* EVEX Byte2 */
 /* XOP bit fields */
 #define X86_XOP_R(xop)	((xop) & 0x80)	/* XOP Byte2 */
 #define X86_XOP_X(xop)	((xop) & 0x40)	/* XOP Byte2 */
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1689,33 +1689,62 @@ enum insn_mmio_type insn_decode_mmio(str
  */
 bool insn_is_nop(struct insn *insn)
 {
-	u8 rex, rex_b = 0, rex_x = 0, rex_r = 0, rex_w = 0;
+	u8 b3 = 0, x3 = 0, r3 = 0, w = 0;
+	u8 b4 = 0, x4 = 0, r4 = 0;
 	u8 modrm, modrm_mod, modrm_reg, modrm_rm;
 	u8 sib = 0, sib_scale, sib_index, sib_base;
 	insn_byte_t p;
+	u8 nrex, rex;
 	int i;
 
-	if (insn->rex_prefix.nbytes) {
-		rex = insn->rex_prefix.bytes[0];
-		rex_w = !!X86_REX_W(rex);
-		rex_r = !!X86_REX_R(rex);
-		rex_x = !!X86_REX_X(rex);
-		rex_b = !!X86_REX_B(rex);
+	if ((nrex = insn->rex_prefix.nbytes)) {
+		rex = insn->rex_prefix.bytes[nrex-1];
+
+		w  = !!X86_REX_W(rex);
+		r3 = !!X86_REX_R(rex);
+		x3 = !!X86_REX_X(rex);
+		b3 = !!X86_REX_B(rex);
+		if (nrex > 1) {
+			r4 = !!X86_REX2_R(rex);
+			x4 = !!X86_REX2_X(rex);
+			b4 = !!X86_REX2_B(rex);
+		}
+
+	} else switch (insn->vex_prefix.nbytes) {
+	case 2: /* VEX2 */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		break;
+	case 3: /* VEX3 */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		x3 =  !X86_VEX_X(insn->vex_prefix.bytes[1]);
+		b3 =  !X86_VEX_B(insn->vex_prefix.bytes[1]);
+		w  = !!X86_VEX_W(insn->vex_prefix.bytes[2]);
+		break;
+	case 4: /* EVEX */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		x3 =  !X86_VEX_X(insn->vex_prefix.bytes[1]);
+		b3 =  !X86_VEX_B(insn->vex_prefix.bytes[1]);
+		w  = !!X86_VEX_W(insn->vex_prefix.bytes[2]);
+		r4 =  !X86_EVEX_R4(insn->vex_prefix.bytes[1]);
+		b4 = !!X86_EVEX_B4(insn->vex_prefix.bytes[1]);
+		x4 =  !X86_EVEX_X4(insn->vex_prefix.bytes[2]);
+		break;
+	default: break;
 	}
 
 	if (insn->modrm.nbytes) {
 		modrm = insn->modrm.bytes[0];
 		modrm_mod = X86_MODRM_MOD(modrm);
-		modrm_reg = X86_MODRM_REG(modrm) + 8*rex_r;
-		modrm_rm  = X86_MODRM_RM(modrm)  + 8*rex_b;
+		modrm_reg = X86_MODRM_REG(modrm) + 8*r3 + 16*r4;
+		modrm_rm  = X86_MODRM_RM(modrm)  + 8*b3 + 16*b4;
 		modrm = 1;
 	}
 
 	if (insn->sib.nbytes) {
 		sib = insn->sib.bytes[0];
 		sib_scale = X86_SIB_SCALE(sib);
-		sib_index = X86_SIB_INDEX(sib) + 8*rex_x;
-		sib_base  = X86_SIB_BASE(sib)  + 8*rex_b;
+		sib_index = X86_SIB_INDEX(sib) + 8*x3 + 16*x4;
+		sib_base  = X86_SIB_BASE(sib)  + 8*b3 + 16*b4;
 		sib = 1;
 
 		modrm_rm = sib_base;
@@ -1729,7 +1758,7 @@ bool insn_is_nop(struct insn *insn)
 		if (modrm_mod != 3) /* register-direct */
 			return false;
 
-		if (insn->x86_64 && !rex_w) /* native size */
+		if (insn->x86_64 && !w) /* native size */
 			return false;
 
 		for_each_insn_prefix(insn, i, p) {
@@ -1743,7 +1772,7 @@ bool insn_is_nop(struct insn *insn)
 		if (modrm_mod == 0 || modrm_mod == 3) /* register-indirect with disp */
 			return false;
 
-		if (insn->x86_64 && !rex_w) /* native size */
+		if (insn->x86_64 && !w) /* native size */
 			return false;
 
 		if (insn->displacement.value != 0)
@@ -1760,7 +1789,7 @@ bool insn_is_nop(struct insn *insn)
 		return modrm_reg == modrm_rm; /* LEA 0(%reg), %reg */
 
 	case 0x90: /* NOP */
-		if (rex_b) /* XCHG %r8,%rax */
+		if (b3 || b4) /* XCHG %r8,%rax */
 			return false;
 
 		for_each_insn_prefix(insn, i, p) {

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

[Alexandre Chartre]

On 9/25/25 12:42, Peter Zijlstra wrote:
> On Thu, Sep 25, 2025 at 12:03:23PM +0200, Peter Zijlstra wrote:
> 
>>>>>>     	case 0x90:
>>>>>> +		if (prefix != 0xf3) /* REP NOP := PAUSE */
>>>>>> +			insn->type = INSN_NOP;
>>>>>>     		break;
> 
>>> What about 0x49 0x90, which is xchg (XCHG r8,rAX) ?
> 
> I've made that:
> 
> 	case 0x90:
> 		if (rex_b) /* XCHG %r8, %rax */
> 			break;
> 
> 		if (prefix == 0xf3) /* REP NOP := PAUSE */
> 			break;
> 
> 		insn->type = INSN_NOP;
> 		break;
> 
> 

Sounds good.

Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

alex.

[tip: objtool/core] objtool/x86: Fix NOP decode

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     044f721ccd33103349eebbb960825584bc6d8e23
Gitweb:        https://git.kernel.org/tip/044f721ccd33103349eebbb960825584bc6d8e23
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 24 Sep 2025 15:27:03 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 14 Oct 2025 13:43:11 +02:00

objtool/x86: Fix NOP decode

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 | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index ef6e96d..204e2ad 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -494,6 +494,12 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 		break;
 
 	case 0x90:
+		if (rex_b) /* XCHG %r8, %rax */
+			break;
+
+		if (prefix == 0xf3) /* REP NOP := PAUSE */
+			break;
+
 		insn->type = INSN_NOP;
 		break;
 
@@ -547,13 +553,14 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 
 		} else if (op2 == 0x0b || op2 == 0xb9) {
 
-			/* ud2 */
+			/* ud2, ud1 */
 			insn->type = INSN_BUG;
 
-		} else if (op2 == 0x0d || op2 == 0x1f) {
+		} else if (op2 == 0x1f) {
 
-			/* nopl/nopw */
-			insn->type = INSN_NOP;
+			/* 0f 1f /0 := NOPL */
+			if (modrm_reg == 0)
+				insn->type = INSN_NOP;
 
 		} else if (op2 == 0x1e) {