x86, cpu: Clean up and unify the NOP selection infrastructure

RFC PATCH New H. Peter Anvin 2 participants 11 messages Apr 18, 2011 – Apr 19, 2011

Subsystems (4)

x86 ftrace Per-CPU memory allocator Static branch/call

lore

Share & tools

lore

H. Peter Anvin

author

2011-04-18 22:57 UTC #0 lore raw

RFC PATCH 1/1 x86, cpu: Clean up and unify the NOP selection infrastructure

This is a RFC patchset to clean up and unify the NOP selection. In particular, this patchset gets rid of data structures built using assembly, moves the selection of an atomic 5-byte NOP into the same infrastructure as everything else, and does some small amount of 32- vs. 64-bit unification.

Comments appreciated.

[PATCH 1/3] x86, percpu: Use ASM_NOP4 instead of hardcoding P6_NOP4 [PATCH 2/3] x86, cpu: Clean up and unify the NOP selection infrastructure [PATCH 3/3] x86, cpu: Change NOP selection for certain Intel CPUs

H. Peter Anvin

author

2011-04-18 22:57 UTC #1 lore raw

PATCH 1/3 x86, percpu: Use ASM_NOP4 instead of hardcoding P6_NOP4

For use in assembly constants, use the ASM_NOP* defines.

Signed-off-by H. Peter Anvin <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Tejun Heo <[email protected]>

---
 arch/x86/include/asm/percpu.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/include/asm/percpu.h +1 -1

index d475b43..751e7f3 100644
@@ -517,7 +517,7 @@ do {									\
 	typeof(o2) __o2 = o2;						\
 	typeof(o2) __n2 = n2;						\
 	typeof(o2) __dummy;						\
-	alternative_io("call this_cpu_cmpxchg16b_emu\n\t" P6_NOP4,	\
+	alternative_io("call this_cpu_cmpxchg16b_emu\n\t" ASM_NOP4,	\
 		       "cmpxchg16b " __percpu_prefix "(%%rsi)\n\tsetz %0\n\t",	\
 		       X86_FEATURE_CX16,				\
 		       ASM_OUTPUT2("=a"(__ret), "=d"(__dummy)),		\

Signature · 1 line

1.7.3.4

tip-bot for H. Peter Anvin

author

2011-04-19 01:01 UTC #2 lore raw

[tip:x86/cpu] x86, percpu: Use ASM_NOP4 instead of hardcoding P6_NOP4

Commit-ID:  b1e7734f024c9ce4393016a97c8d821e1f18d9b4
Gitweb:     http://git.kernel.org/tip/b1e7734f024c9ce4393016a97c8d821e1f18d9b4
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:18:02 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 16:40:06 -0700

x86, percpu: Use ASM_NOP4 instead of hardcoding P6_NOP4

For use in assembly constants, use the ASM_NOP* defines.

Signed-off-by H. Peter Anvin <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Tejun Heo <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 arch/x86/include/asm/percpu.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/include/asm/percpu.h +1 -1

index d475b43..751e7f3 100644
@@ -517,7 +517,7 @@ do {									\
 	typeof(o2) __o2 = o2;						\
 	typeof(o2) __n2 = n2;						\
 	typeof(o2) __dummy;						\
-	alternative_io("call this_cpu_cmpxchg16b_emu\n\t" P6_NOP4,	\
+	alternative_io("call this_cpu_cmpxchg16b_emu\n\t" ASM_NOP4,	\
 		       "cmpxchg16b " __percpu_prefix "(%%rsi)\n\tsetz %0\n\t",	\
 		       X86_FEATURE_CX16,				\
 		       ASM_OUTPUT2("=a"(__ret), "=d"(__dummy)),		\

H. Peter Anvin

author

2011-04-18 22:57 UTC #3 lore raw

PATCH 2/3 x86, cpu: Clean up and unify the NOP selection infrastructure

In reply to H. Peter Anvin

Clean up and unify the NOP selection infrastructure:

- Make the atomic 5-byte NOP a part of the selection system.
- Pick NOPs once during early boot and then be done with it.

Signed-off-by H. Peter Anvin <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jason Baron <[email protected]>

---
 arch/x86/include/asm/alternative.h |    8 --
 arch/x86/include/asm/nops.h        |  146 ++++++++++++++++++--------------
 arch/x86/kernel/alternative.c      |  165 +++++++++++++++++++-----------------
 arch/x86/kernel/ftrace.c           |    4 +-
 arch/x86/kernel/jump_label.c       |    5 +-
 arch/x86/kernel/setup.c            |    6 +-
 6 files changed, 177 insertions(+), 157 deletions(-)

arch/x86/include/asm/alternative.h +0 -8

index 13009d1..7da1682 100644
@@ -191,12 +191,4 @@ extern void *text_poke(void *addr, const void *opcode, size_t len);
 extern void *text_poke_smp(void *addr, const void *opcode, size_t len);
 extern void text_poke_smp_batch(struct text_poke_param *params, int n);
 
-#if defined(CONFIG_DYNAMIC_FTRACE) || defined(HAVE_JUMP_LABEL)
-#define IDEAL_NOP_SIZE_5 5
-extern unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
-extern void arch_init_ideal_nop5(void);
-#else
-static inline void arch_init_ideal_nop5(void) {}
-#endif
-
 #endif /* _ASM_X86_ALTERNATIVE_H */

arch/x86/include/asm/nops.h +84 ~~-62~~

183 lines not on this page · raw message

arch/x86/kernel/alternative.c +87 ~~-78~~

236 lines not on this page · raw message

arch/x86/kernel/ftrace.c +2 -2

index a93742a..0ba15a6 100644
@@ -260,9 +260,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code)
 	return mod_code_status;
 }
 
-static unsigned char *ftrace_nop_replace(void)
+static const unsigned char *ftrace_nop_replace(void)
 {
-	return ideal_nop5;
+	return ideal_nops[NOP_ATOMIC5];
 }
 
 static int

arch/x86/kernel/jump_label.c +3 -2

index 961b6b3..3fee346 100644
@@ -34,7 +34,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 		code.offset = entry->target -
 				(entry->code + JUMP_LABEL_NOP_SIZE);
 	} else
-		memcpy(&code, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
 	get_online_cpus();
 	mutex_lock(&text_mutex);
 	text_poke_smp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);

@@ -44,7 +44,8 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 void arch_jump_label_text_poke_early(jump_label_t addr)
 {
-	text_poke_early((void *)addr, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+	text_poke_early((void *)addr, ideal_nops[NOP_ATOMIC5],
+			JUMP_LABEL_NOP_SIZE);
 }
 
 #endif

arch/x86/kernel/setup.c +1 -5

index 5a0484a..390a663 100644
@@ -691,8 +691,6 @@ early_param("reservelow", parse_reservelow);
 
 void __init setup_arch(char **cmdline_p)
 {
-	unsigned long flags;
-
 #ifdef CONFIG_X86_32
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 	visws_early_detect();

@@ -1036,9 +1034,7 @@ void __init setup_arch(char **cmdline_p)
 
 	mcheck_init();
 
-	local_irq_save(flags);
-	arch_init_ideal_nop5();
-	local_irq_restore(flags);
+	arch_init_ideal_nops();
 }
 
 #ifdef CONFIG_X86_32

Signature · 1 line

1.7.3.4

Steven Rostedt

maintainer

2011-04-18 23:31 UTC #4 lore raw

Quoted 7 lines · H. Peter Anvin

#endif /* _ASM_X86_NOPS_H */

arch/x86/kernel/alternative.c +0 -0

index 4a23467..2ca3f65 100644
@@ -68,16 +68,20 @@ __setup("noreplace-paravirt", setup_noreplace_paravirt);
 	printk(KERN_DEBUG fmt, args)

arch/x86/kernel/alternative.c __setup("noreplace-paravirt", setup_noreplace_paravirt); · line 78

Quoted 10 lines

arch/x86/kernel/alternative.c +0 -8

 
 #if defined(GENERIC_NOP1) && !defined(CONFIG_X86_64)
-/* Use inline assembly to define this because the nops are defined
-   as inline assembly strings in the include files and we cannot
-   get them easily into strings. */
-asm("\t" __stringify(__INITRODATA_OR_MODULE) "\nintelnops: "
-	GENERIC_NOP1 GENERIC_NOP2 GENERIC_NOP3 GENERIC_NOP4 GENERIC_NOP5 GENERIC_NOP6
-	GENERIC_NOP7 GENERIC_NOP8
-    "\t.previous");
-extern const unsigned char intelnops[];

Can we please add a comment to this. The original (above) was confusing enough, but at least it used asm() so it wasn't that bad to figure out. Or at least the asm() usage would trigger in one's mind to think "Damn! They chose to use 'asm', it must be some kind of nasty trick. Let's take a better look at WTF they are doing!".

Now the use a normal character array actual makes this even more subtle. What about adding:

/*
* Each GENERIC_NOPX is of X bytes, and defined as an array of bytes
* that correspond to that nop. Getting from one nop to the next, we
* add to the array the offset that is equal to the sum of all sizes of
* nops preceding the one we are after.
*
* Note: The GENERIC_NOP5_ATOMIC is at the end, as it breaks the
* nice symmetry of sizes of the previous nops.
*/

-- Steve

Quoted 25 lines

arch/x86/kernel/alternative.c +14 -1

+static const unsigned char  __initconst_or_module intelnops[] =
+{
+	GENERIC_NOP1,
+	GENERIC_NOP2,
+	GENERIC_NOP3,
+	GENERIC_NOP4,
+	GENERIC_NOP5,
+	GENERIC_NOP6,
+	GENERIC_NOP7,
+	GENERIC_NOP8,
+	GENERIC_NOP5_ATOMIC
+};
 static const unsigned char *const __initconst_or_module
-intel_nops[ASM_NOP_MAX+1] = {
+intel_nops[ASM_NOP_MAX+2] = {
 	NULL,
 	intelnops,
 	intelnops + 1,

@@ -87,17 +91,25 @@ intel_nops[ASM_NOP_MAX+1] = {
 	intelnops + 1 + 2 + 3 + 4 + 5,
 	intelnops + 1 + 2 + 3 + 4 + 5 + 6,
 	intelnops + 1 + 2 + 3 + 4 + 5 + 6 + 7,
+	intelnops + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8,
 };
 #endif

H. Peter Anvin

author

2011-04-18 23:39 UTC #5 lore raw

Quoted 8 lines · Steven Rostedt

Can we please add a comment to this. The original (above) was confusing enough, but at least it used asm() so it wasn't that bad to figure out. Or at least the asm() usage would trigger in one's mind to think "Damn! They chose to use 'asm', it must be some kind of nasty trick. Let's take a better look at WTF they are doing!".
Now the use a normal character array actual makes this even more subtle.

OK... I never thought it was particularly subtle, but okay.

A much bigger issue with this particular patch is that the __init{data,const}_or_module presumably needs to be removed from these structures, right?

-hpa

Steven Rostedt

maintainer

2011-04-18 23:58 UTC #6 lore raw

Quoted 11 lines · H. Peter Anvin

Quoted 8 lines · Steven Rostedt
Can we please add a comment to this. The original (above) was confusing enough, but at least it used asm() so it wasn't that bad to figure out. Or at least the asm() usage would trigger in one's mind to think "Damn! They chose to use 'asm', it must be some kind of nasty trick. Let's take a better look at WTF they are doing!".
Now the use a normal character array actual makes this even more subtle.
OK... I never thought it was particularly subtle, but okay.

It took me 2 minutes to figure out what it was doing, but then maybe I'm slow ;) But having a comment may save a minute or two of frustration from other reviewers as well.

Quoted 4 lines

A much bigger issue with this particular patch is that the __init{data,const}_or_module presumably needs to be removed from these structures, right?

Ah, as you have this:

Quoted 4 lines

arch/x86/kernel/ftrace.c +0 -0

@@ -260,9 +260,9 @@ do_ftrace_mod_code(unsigned long ip, void
 *new_code)

arch/x86/kernel/ftrace.c do_ftrace_mod_code(unsigned long ip, void · line 268

Quoted 10 lines

arch/x86/kernel/ftrace.c +2 -2

        return mod_code_status;
 }
 
-static unsigned char *ftrace_nop_replace(void)
+static const unsigned char *ftrace_nop_replace(void)
 {
-       return ideal_nop5;
+       return ideal_nops[NOP_ATOMIC5];
 }

I would say, yes get rid of the init annotations.

-- Steve

tip-bot for H. Peter Anvin

author

2011-04-19 01:01 UTC #7 lore raw

In reply to H. Peter Anvin

Commit-ID:  dc326fca2b640fc41aed7c015d0f456935a66255
Gitweb:     http://git.kernel.org/tip/dc326fca2b640fc41aed7c015d0f456935a66255
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:19:51 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 16:40:21 -0700

x86, cpu: Clean up and unify the NOP selection infrastructure

Clean up and unify the NOP selection infrastructure:

- Make the atomic 5-byte NOP a part of the selection system.
- Pick NOPs once during early boot and then be done with it.

Signed-off-by H. Peter Anvin <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jason Baron <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 arch/x86/include/asm/alternative.h |    8 --
 arch/x86/include/asm/nops.h        |  146 +++++++++++++++++------------
 arch/x86/kernel/alternative.c      |  182 ++++++++++++++++++++----------------
 arch/x86/kernel/ftrace.c           |    4 +-
 arch/x86/kernel/jump_label.c       |    5 +-
 arch/x86/kernel/setup.c            |    6 +-
 6 files changed, 190 insertions(+), 161 deletions(-)

arch/x86/include/asm/alternative.h +0 -8

index 13009d1..7da1682 100644
@@ -191,12 +191,4 @@ extern void *text_poke(void *addr, const void *opcode, size_t len);
 extern void *text_poke_smp(void *addr, const void *opcode, size_t len);
 extern void text_poke_smp_batch(struct text_poke_param *params, int n);
 
-#if defined(CONFIG_DYNAMIC_FTRACE) || defined(HAVE_JUMP_LABEL)
-#define IDEAL_NOP_SIZE_5 5
-extern unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
-extern void arch_init_ideal_nop5(void);
-#else
-static inline void arch_init_ideal_nop5(void) {}
-#endif
-
 #endif /* _ASM_X86_ALTERNATIVE_H */

arch/x86/include/asm/nops.h +84 ~~-62~~

183 lines not on this page · raw message

arch/x86/kernel/alternative.c +100 ~~-82~~

250 lines not on this page · raw message

arch/x86/kernel/ftrace.c +2 -2

index a93742a..0ba15a6 100644
@@ -260,9 +260,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code)
 	return mod_code_status;
 }
 
-static unsigned char *ftrace_nop_replace(void)
+static const unsigned char *ftrace_nop_replace(void)
 {
-	return ideal_nop5;
+	return ideal_nops[NOP_ATOMIC5];
 }
 
 static int

arch/x86/kernel/jump_label.c +3 -2

index 961b6b3..3fee346 100644
@@ -34,7 +34,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 		code.offset = entry->target -
 				(entry->code + JUMP_LABEL_NOP_SIZE);
 	} else
-		memcpy(&code, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
 	get_online_cpus();
 	mutex_lock(&text_mutex);
 	text_poke_smp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);

@@ -44,7 +44,8 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 void arch_jump_label_text_poke_early(jump_label_t addr)
 {
-	text_poke_early((void *)addr, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+	text_poke_early((void *)addr, ideal_nops[NOP_ATOMIC5],
+			JUMP_LABEL_NOP_SIZE);
 }
 
 #endif

arch/x86/kernel/setup.c +1 -5

index 5a0484a..390a663 100644
@@ -691,8 +691,6 @@ early_param("reservelow", parse_reservelow);
 
 void __init setup_arch(char **cmdline_p)
 {
-	unsigned long flags;
-
 #ifdef CONFIG_X86_32
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 	visws_early_detect();

@@ -1036,9 +1034,7 @@ void __init setup_arch(char **cmdline_p)
 
 	mcheck_init();
 
-	local_irq_save(flags);
-	arch_init_ideal_nop5();
-	local_irq_restore(flags);
+	arch_init_ideal_nops();
 }
 
 #ifdef CONFIG_X86_32

H. Peter Anvin

author

2011-04-18 22:57 UTC #8 lore raw

PATCH 3/3 x86, cpu: Change NOP selection for certain Intel CPUs

In reply to H. Peter Anvin

Due to a decoder implementation quirk, some specific Intel CPUs actually perform better with the "k8_nops" than with the SDM-recommended NOPs. For runtime-selected NOPs, if we detect those specific CPUs then use the k8_nops instead of the ones we would normally use.

Signed-off-by H. Peter Anvin <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jason Baron <[email protected]>

---
 arch/x86/kernel/alternative.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

arch/x86/kernel/alternative.c +8 -1

index 2ca3f65..34bf87e 100644
@@ -190,7 +190,14 @@ void __init arch_init_ideal_nops(void)
 {
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:
-		if (boot_cpu_has(X86_FEATURE_NOPL)) {
+		if (boot_cpu_data.x86 == 6 &&
+		    boot_cpu_data.x86_model >= 0x0f &&
+		    boot_cpu_data.x86_model != 0x1c &&
+		    boot_cpu_data.x86_model != 0x26 &&
+		    boot_cpu_data.x86_model != 0x27 &&
+		    boot_cpu_data.x86_model < 0x30) {
+			ideal_nops = k8_nops;
+		} else if (boot_cpu_has(X86_FEATURE_NOPL)) {
 			   ideal_nops = p6_nops;
 		} else {
 #ifdef CONFIG_X86_64

Signature · 1 line

1.7.3.4

Steven Rostedt

maintainer

2011-04-18 23:34 UTC #9 lore raw

Re: [PATCH 3/3] x86, cpu: Change NOP selection for certain Intel CPUs

Quoted 20 lines · H. Peter Anvin

Due to a decoder implementation quirk, some specific Intel CPUs actually perform better with the "k8_nops" than with the SDM-recommended NOPs. For runtime-selected NOPs, if we detect those specific CPUs then use the k8_nops instead of the ones we would normally use.
Signed-off-by H. Peter Anvin <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jason Baron <[email protected]>
---
 arch/x86/kernel/alternative.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
arch/x86/kernel/alternative.c +0 -0
index 2ca3f65..34bf87e 100644

arch/x86/kernel/alternative.c void __init arch_init_ideal_nops(void) · line 192

Quoted 3 lines

arch/x86/kernel/alternative.c +0 -0

@@ -190,7 +190,14 @@ void __init arch_init_ideal_nops(void)
 {
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:

Please add:

/*
* Due to a decoder implementation quirk, some specific Intel CPUs
* actually perform better with the "k8_nops" than with the
* SDM-recommended NOPs. For runtime-selected NOPs, if we detect those
* specific CPUs then use the k8_nops instead of the ones we would
* normally use.
*/

I tend to read comments by the code more than change logs that added the code.

Thanks,

-- Steve

Quoted 12 lines

arch/x86/kernel/alternative.c +8 -1

-		if (boot_cpu_has(X86_FEATURE_NOPL)) {
+		if (boot_cpu_data.x86 == 6 &&
+		    boot_cpu_data.x86_model >= 0x0f &&
+		    boot_cpu_data.x86_model != 0x1c &&
+		    boot_cpu_data.x86_model != 0x26 &&
+		    boot_cpu_data.x86_model != 0x27 &&
+		    boot_cpu_data.x86_model < 0x30) {
+			ideal_nops = k8_nops;
+		} else if (boot_cpu_has(X86_FEATURE_NOPL)) {
 			   ideal_nops = p6_nops;
 		} else {
 #ifdef CONFIG_X86_64

tip-bot for H. Peter Anvin

author

2011-04-19 01:02 UTC #10 lore raw

[tip:x86/cpu] x86, cpu: Change NOP selection for certain Intel CPUs

In reply to H. Peter Anvin

Commit-ID:  d8d9766c8c29f71c37bc4b74cc9fcf6a192c9bfd
Gitweb:     http://git.kernel.org/tip/d8d9766c8c29f71c37bc4b74cc9fcf6a192c9bfd
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:31:57 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 16:40:32 -0700

x86, cpu: Change NOP selection for certain Intel CPUs

Signed-off-by H. Peter Anvin <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jason Baron <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 arch/x86/kernel/alternative.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

arch/x86/kernel/alternative.c +13 -1

index 846f61e..c0501ea 100644
@@ -199,7 +199,19 @@ void __init arch_init_ideal_nops(void)
 {
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:
-		if (boot_cpu_has(X86_FEATURE_NOPL)) {
+		/*
+		 * Due to a decoder implementation quirk, some
+		 * specific Intel CPUs actually perform better with
+		 * the "k8_nops" than with the SDM-recommended NOPs.
+		 */
+		if (boot_cpu_data.x86 == 6 &&
+		    boot_cpu_data.x86_model >= 0x0f &&
+		    boot_cpu_data.x86_model != 0x1c &&
+		    boot_cpu_data.x86_model != 0x26 &&
+		    boot_cpu_data.x86_model != 0x27 &&
+		    boot_cpu_data.x86_model < 0x30) {
+			ideal_nops = k8_nops;
+		} else if (boot_cpu_has(X86_FEATURE_NOPL)) {
 			   ideal_nops = p6_nops;
 		} else {
 #ifdef CONFIG_X86_64