x86/jump label: Paranoid checks and 2 or 5 byte nops

Ingo,

I've done some final clean ups of the 2 or 5 byte nop code and I also added a few generic clean ups of jump label itself. The first 2 patches use the best nops as defined by Peter when we do not know what the machine is running.

The third patch adds paranoid checks, to make sure what we are modifying is truely what it expects. Otherwise we bug and crash the kernel (as recommended by Peter).

The last two patches are the clean up versions of the 2 or 5 byte nops.

Note, there is code in the infrastructure that can be shared among other users in scripts and elsewhere. I rather get this code in first and then fiddle with seeing who does what and then integrate all the code together. As that may not be such an easy task, and I don't want to post pone this code due to it.

Also, I feel that we can let this code sit for a bit on LKML and let people comment. If everyone gives there acks, then we can pull this code in. I already tested it, but if there are acks, I can rebase just to update the acked-bys.

Please pull (if everyone's OK with it) the latest tip/perf/jump-label tree,
which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/jump-label

Head SHA1: 7837dddecaa7e7f31b732a0c302a4bdd255a8321

Steven Rostedt (5):
x86/jump-label: Use best default nops for inital jump label calls x86/jump-label: Do not bother updating nops if they are correct x86/jump-label: Add safety checks to jump label conversions jump labels: Add infrastructure to update jump labels at compile time jump labels/x86: Use etiher 5 byte or 2 byte jumps

----

 Makefile                          |    7 +
 arch/Kconfig                      |    6 +
 arch/x86/Kconfig                  |    1 +
 arch/x86/include/asm/jump_label.h |    9 +-
 arch/x86/kernel/jump_label.c      |  110 +++++++++++-
 scripts/Makefile                  |    1 +
 scripts/Makefile.build            |   15 ++-
 scripts/update_jump_label.c       |  335 +++++++++++++++++++++++++++++++++++++
 scripts/update_jump_label.h       |  208 +++++++++++++++++++++++
 9 files changed, 678 insertions(+), 14 deletions(-)

From: Steven Rostedt <[email protected]>

As specified by H. Peter Anvin, the best nops for x86 without knowing the running computer is:

32bit:
0x3e, 0x8d, 0x74, 0x26, 0x00

64bit:
0x0f, 0x1f, 0x44, 0x00, 0x00 also known as P6_NOP5

Currently the default nop that is used by jump label is:

0xe9 0x00 0x00 0x00 0x00

Which is really a 5byte jump to the next position.

It's better to use a real nop than a jmp.

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

index a32b18c..cefcfd3 100644
@@ -3,18 +3,23 @@
 
 #ifdef __KERNEL__
 
+#include <linux/stringify.h>
 #include <linux/types.h>
 #include <asm/nops.h>
 #include <asm/asm.h>
 
 #define JUMP_LABEL_NOP_SIZE 5
 
-#define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
+#ifdef CONFIG_X86_64
+# define JUMP_LABEL_INIT_NOP P6_NOP5
+#else
+# define JUMP_LABEL_INIT_NOP 0x3e, 0x8d, 0x74, 0x26, 0x00
+#endif
 
 static __always_inline bool arch_static_branch(struct jump_label_key *key)
 {
 	asm goto("1:"
-		JUMP_LABEL_INITIAL_NOP
+		".byte " __stringify(JUMP_LABEL_INIT_NOP) "\n\t"
 		".pushsection __jump_table,  \"aw\" \n\t"
 		_ASM_ALIGN "\n\t"
 		_ASM_PTR "1b, %l[l_yes], %c0 \n\t"

1.7.8.3

[ Added Michal ]

This eventually needs to match the code in the update_jump_label.c, otherwise the checks will be incorrect on boot up, and we'll BUG the kernel.

I need to find a way to get the config options of the kernel into the compiling of the update_jump_label.c. Then I could use the nops.h file with the set configs.

Michal, can we get host tools to compile with the configs set by the .config file?

update_jump_label.c is built just like recordmcount.c is. It would be great if it could be compiled with the same config.h defines as the kernel is. That way I can make it (and even recordmcount.c) customize to the type of kernel that is being built.

Then again, it would also need to be part of the dependencies that are done with configs.

If not, at the very least, I'll add a comment (in the last patch) that explains why we use our own "JUMP_LABEL_INIT_NOP" instead of ASM_NOP5_ATOMIC.

-- Steve

Then at least use GENERIC_NOP5_ATOMIC instead of open-coding it.

-hpa

OK, I can do that. Hehe, I looked for the x86_32 version, but it was so obfuscated that I didn't find it. Thanks for pointing it out.

-- Steve

You can do a #include "../include/generated/autoconf.h" and you will get the CONFIG_ definitions.

Yeah, we should simply compile all the host tools with autoconf.h and kconfig.h included. The idea behind not doing this probably was that a single scripts/ directory should work with multiple kernel configurations, but this is obviously not the case.

Michal

From: Steven Rostedt <[email protected]>

On boot up, the jump label init function scans all the jump label locations and converts them to the best nop for the machine. If the nop is already the ideal nop, do not bother with changing it.

---
 arch/x86/kernel/jump_label.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

index ea9d5f2f..0f4c6ba 100644
@@ -53,7 +53,25 @@ void arch_jump_label_transform(struct jump_entry *entry,
 void arch_jump_label_transform_static(struct jump_entry *entry,
 				      enum jump_label_type type)
 {
-	__jump_label_transform(entry, type, text_poke_early);
+	static int once;
+	static int update;
+
+	/*
+	 * This function is called at boot up and when modules are
+	 * first loaded. Check if the default nop, the one that is
+	 * inserted at compile time, is the ideal nop. If it is, then
+	 * we do not need to update the nop, and we can leave it as is.
+	 * If it is not, then we need to update the nop to the ideal nop.
+	 */
+	if (!once) {
+		unsigned char default_nop[] = { JUMP_LABEL_INIT_NOP };
+		const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
+		once++;
+		if (memcmp(ideal_nop, default_nop, 5) != 0)
+			update = 1;
+	}
+	if (update)
+		__jump_label_transform(entry, type, text_poke_early);
 }
 
 #endif

1.7.8.3

OK will update,

Thanks!

-- Steve

From: Steven Rostedt <[email protected]>

As with all modifying of kernel text, we need to be very paranoid.

When converting the jump label locations to and from nops to jumps a check has been added to make sure what we are replacing is what we expect, otherwise we bug.

---
 arch/x86/kernel/jump_label.c |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

index 0f4c6ba..44f2528 100644
@@ -26,16 +26,40 @@ union jump_code_union {
 
 static void __jump_label_transform(struct jump_entry *entry,
 				   enum jump_label_type type,
-				   void *(*poker)(void *, const void *, size_t))
+				   void *(*poker)(void *, const void *, size_t),
+				   int init)
 {
 	union jump_code_union code;
+	const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
 
 	if (type == JUMP_LABEL_ENABLE) {
+		/*
+		 * We are enabling this jump label. If it is not a nop
+		 * then something must have gone wrong.
+		 */
+		BUG_ON(memcmp((void *)entry->code, ideal_nop, 5) != 0);
+
 		code.jump = 0xe9;
 		code.offset = entry->target -
 				(entry->code + JUMP_LABEL_NOP_SIZE);
-	} else
+	} else {
+		/*
+		 * We are disabling this jump label. If it is not what
+		 * we think it is, then something must have gone wrong.
+		 * If this is the first initialization call, then we
+		 * are converting the default nop to the ideal nop.
+		 */
+		if (init) {
+			unsigned char default_nop[] = { JUMP_LABEL_INIT_NOP };
+			BUG_ON(memcmp((void *)entry->code, default_nop, 5) != 0);
+		} else {
+			code.jump = 0xe9;
+			code.offset = entry->target -
+				(entry->code + JUMP_LABEL_NOP_SIZE);
+			BUG_ON(memcmp((void *)entry->code, &code, 5) != 0);
+		}
 		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+	}
 
 	(*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
 }

@@ -45,7 +69,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 {
 	get_online_cpus();
 	mutex_lock(&text_mutex);
-	__jump_label_transform(entry, type, text_poke_smp);
+	__jump_label_transform(entry, type, text_poke_smp, 0);
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
 }

@@ -71,7 +95,7 @@ void arch_jump_label_transform_static(struct jump_entry *entry,
 			update = 1;
 	}
 	if (update)
-		__jump_label_transform(entry, type, text_poke_early);
+		__jump_label_transform(entry, type, text_poke_early, 1);
 }
 
 #endif

1.7.8.3

From: Steven Rostedt <[email protected]>

Add the infrastructure to allow architectures to modify the jump label locations at compile time. This is mainly for x86, where the jmps may be either 2 bytes or 5 bytes. Instead of wasting 5 bytes for all jump labels, this code will let x86 put in a jmp instead of a 5 byte nop. Then the assembler will make either a 2 byte or 5 byte jmp depending on where the target is.

At compile time, this code will replace the jmps with either a 2 byte or 5 byte nop depending on the size of the jmp that was added.

---
 Makefile                    |    7 +
 arch/Kconfig                |    6 +
 scripts/Makefile            |    1 +
 scripts/Makefile.build      |   15 ++-
 scripts/update_jump_label.c |  335 +++++++++++++++++++++++++++++++++++++++++++
 scripts/update_jump_label.h |  208 +++++++++++++++++++++++++++
 6 files changed, 570 insertions(+), 2 deletions(-)
 create mode 100644 scripts/update_jump_label.c
 create mode 100644 scripts/update_jump_label.h

index adddd11..2b3fdf3 100644
@@ -611,6 +611,13 @@ ifdef CONFIG_DYNAMIC_FTRACE
 endif
 endif
 
+ifdef CONFIG_JUMP_LABEL
+	ifdef CONFIG_HAVE_BUILD_TIME_JUMP_LABEL
+		BUILD_UPDATE_JUMP_LABEL := y
+		export BUILD_UPDATE_JUMP_LABEL
+	endif
+endif
+
 # We trigger additional mismatches with less inlining
 ifdef CONFIG_DEBUG_SECTION_MISMATCH
 KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)

index 4b0669c..8fa6934 100644
@@ -169,6 +169,12 @@ config HAVE_PERF_EVENTS_NMI
 	  subsystem.  Also has support for calculating CPU cycle events
 	  to determine how many clock cycles in a given period.
 
+config HAVE_BUILD_TIME_JUMP_LABEL
+       bool
+       help
+	If an arch uses scripts/update_jump_label to patch in jump nops
+	at build time, then it must enable this option.
+
 config HAVE_ARCH_JUMP_LABEL
 	bool
 

index df7678f..738b65c 100644
@@ -13,6 +13,7 @@ hostprogs-$(CONFIG_LOGO)         += pnmtologo
 hostprogs-$(CONFIG_VT)           += conmakehash
 hostprogs-$(CONFIG_IKCONFIG)     += bin2c
 hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
+hostprogs-$(BUILD_UPDATE_JUMP_LABEL) += update_jump_label
 
 always		:= $(hostprogs-y) $(hostprogs-m)
 

index d2b366c..8a84b80 100644
@@ -258,6 +258,15 @@ cmd_modversions =								\
 	fi;
 endif
 
+ifdef BUILD_UPDATE_JUMP_LABEL
+update_jump_label_source := $(srctree)/scripts/update_jump_label.c \
+			$(srctree)/scripts/update_jump_label.h
+cmd_update_jump_label =						\
+	if [ $(@) != "scripts/mod/empty.o" ]; then		\
+		$(objtree)/scripts/update_jump_label "$(@)";	\
+	fi;
+endif
+
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
 ifdef BUILD_C_RECORDMCOUNT
 ifeq ("$(origin RECORDMCOUNT_WARN)", "command line")

@@ -294,6 +303,7 @@ define rule_cc_o_c
 	$(cmd_modversions)						  \
 	$(call echo-cmd,record_mcount)					  \
 	$(cmd_record_mcount)						  \
+	$(cmd_update_jump_label)					  \
 	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
 	                                              $(dot-target).tmp;  \
 	rm -f $(depfile);						  \

@@ -301,13 +311,14 @@ define rule_cc_o_c
 endef
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(update_jump_label_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) \
+		  $(update_jump_label_source) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)

1.7.8.3

From: Steven Rostedt <[email protected]>

Have the jump labels add a "jmp" in the assembly instead of a default nop. This will cause the assembler to put in either a 2 byte or 5 byte jmp depending on where the target lable is.

Then at compile time, the update_jump_label code will replace the jmps with either 2 or 5 byte nops.

On boot up, the code can be examined to see if the jump label uses either a 2 or 5 byte nop and replace it.

By allowing the jump labels to be 2 bytes, it speeds up the nops, not only 2 byte nops are faster than 5 byte nops, but also because it saves on cache foot print.

text data bss dec hex filename
13403667 3666856 2998272 20068795 13239bb ../nobackup/mxtest/vmlinux-old 13398536 3666856 2998272 20063664 13225b0 ../nobackup/mxtest/vmlinux-new

Converting the current v3.2 trace points saved 5,131 bytes. As more places use jump labels, this will have a bigger savings.

---
 arch/x86/Kconfig                  |    1 +
 arch/x86/include/asm/jump_label.h |    2 +-
 arch/x86/kernel/jump_label.c      |   86 ++++++++++++++++++++++++++++--------
 3 files changed, 69 insertions(+), 20 deletions(-)

index efb4294..b5004c1 100644
@@ -61,6 +61,7 @@ config X86
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_BUILD_TIME_JUMP_LABEL
 	select HAVE_TEXT_POKE_SMP
 	select HAVE_GENERIC_HARDIRQS
 	select HAVE_SPARSE_IRQ

index cefcfd3..bde5323 100644
@@ -19,7 +19,7 @@
 static __always_inline bool arch_static_branch(struct jump_label_key *key)
 {
 	asm goto("1:"
-		".byte " __stringify(JUMP_LABEL_INIT_NOP) "\n\t"
+		"jmp %l[l_yes]\n"
 		".pushsection __jump_table,  \"aw\" \n\t"
 		_ASM_ALIGN "\n\t"
 		_ASM_PTR "1b, %l[l_yes], %c0 \n\t"

1.7.8.3