linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] jump_label: get rid of NOP patching where possible
@ 2022-06-15 15:41 Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2022-06-15 15:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ard Biesheuvel, Peter Zijlstra, Mark Rutland,
	Thomas Bogendoerfer, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	linux-mips, linux-s390

The only architecture that actually needs to convert compiler generated
jump label NOP encodings into something else at runtime is MIPS, because
the assembler cannot be trusted to emit a sequence that can be safely
patched into a branch instruction.

All other architectures either do nothing with jump label NOPs at load
time, or patch a perfectly good NOP into a different one, or into the same
one - none of this seems very useful, so let's get rid of it where we
can.

Changes since v1:
- use a default implementation of arch_jump_label_transform_static()
  instead of an empty macro in patch #3
- fix MIPS with CONFIG_MODULES=n
- add acks from Mark and Peter to patch #3

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: linux-mips@vger.kernel.org
Cc: linux-s390@vger.kernel.org

Ard Biesheuvel (3):
  jump_label: s390: avoid pointless initial NOP patching
  jump_label: mips: move module NOP patching into arch code
  jump_label: make initial NOP patching the special case

 Documentation/staging/static-keys.rst |  3 --
 arch/arc/kernel/jump_label.c          | 13 -------
 arch/arm/kernel/jump_label.c          |  6 ---
 arch/arm64/kernel/jump_label.c        | 11 ------
 arch/mips/include/asm/jump_label.h    |  2 +
 arch/mips/kernel/jump_label.c         | 19 +++++++++
 arch/mips/kernel/module.c             |  5 ++-
 arch/parisc/kernel/jump_label.c       | 11 ------
 arch/riscv/kernel/jump_label.c        | 12 ------
 arch/s390/include/asm/jump_label.h    |  5 +--
 arch/s390/kernel/jump_label.c         | 28 +++----------
 arch/s390/kernel/module.c             |  1 -
 arch/sparc/kernel/module.c            |  3 --
 arch/x86/kernel/jump_label.c          | 13 -------
 arch/x86/kernel/module.c              |  3 --
 include/linux/jump_label.h            |  7 +---
 kernel/jump_label.c                   | 41 +++-----------------
 17 files changed, 38 insertions(+), 145 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching
  2022-06-15 15:41 [PATCH v2 0/3] jump_label: get rid of NOP patching where possible Ard Biesheuvel
@ 2022-06-15 15:41 ` Ard Biesheuvel
  2022-06-26  7:57   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
  2 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2022-06-15 15:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ard Biesheuvel, Peter Zijlstra, Mark Rutland,
	Thomas Bogendoerfer, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	linux-mips, linux-s390

Patching NOPs into other NOPs at boot time serves no purpose, so let's
use the same NOP encodings at compile time and runtime.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/s390/include/asm/jump_label.h |  5 ++---
 arch/s390/kernel/jump_label.c      | 23 +++++---------------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
index 916cfcb36d8a..895f774bbcc5 100644
--- a/arch/s390/include/asm/jump_label.h
+++ b/arch/s390/include/asm/jump_label.h
@@ -10,7 +10,6 @@
 #include <linux/stringify.h>
 
 #define JUMP_LABEL_NOP_SIZE 6
-#define JUMP_LABEL_NOP_OFFSET 2
 
 #ifdef CONFIG_CC_IS_CLANG
 #define JUMP_LABEL_STATIC_KEY_CONSTRAINT "i"
@@ -21,12 +20,12 @@
 #endif
 
 /*
- * We use a brcl 0,2 instruction for jump labels at compile time so it
+ * We use a brcl 0,<offset> instruction for jump labels so it
  * can be easily distinguished from a hotpatch generated instruction.
  */
 static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
 {
-	asm_volatile_goto("0:	brcl	0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n"
+	asm_volatile_goto("0:	brcl 0,%l[label]\n"
 			  ".pushsection __jump_table,\"aw\"\n"
 			  ".balign	8\n"
 			  ".long	0b-.,%l[label]-.\n"
diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index 6bec000c6c1c..d764f0d229ab 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -44,14 +44,8 @@ static void jump_label_bug(struct jump_entry *entry, struct insn *expected,
 	panic("Corrupted kernel text");
 }
 
-static struct insn orignop = {
-	.opcode = 0xc004,
-	.offset = JUMP_LABEL_NOP_OFFSET >> 1,
-};
-
 static void jump_label_transform(struct jump_entry *entry,
-				 enum jump_label_type type,
-				 int init)
+				 enum jump_label_type type)
 {
 	void *code = (void *)jump_entry_code(entry);
 	struct insn old, new;
@@ -63,27 +57,22 @@ static void jump_label_transform(struct jump_entry *entry,
 		jump_label_make_branch(entry, &old);
 		jump_label_make_nop(entry, &new);
 	}
-	if (init) {
-		if (memcmp(code, &orignop, sizeof(orignop)))
-			jump_label_bug(entry, &orignop, &new);
-	} else {
-		if (memcmp(code, &old, sizeof(old)))
-			jump_label_bug(entry, &old, &new);
-	}
+	if (memcmp(code, &old, sizeof(old)))
+		jump_label_bug(entry, &old, &new);
 	s390_kernel_write(code, &new, sizeof(new));
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 0);
+	jump_label_transform(entry, type);
 	text_poke_sync();
 }
 
 bool arch_jump_label_transform_queue(struct jump_entry *entry,
 				     enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 0);
+	jump_label_transform(entry, type);
 	return true;
 }
 
@@ -95,6 +84,4 @@ void arch_jump_label_transform_apply(void)
 void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
 						       enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 1);
-	text_poke_sync();
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code
  2022-06-15 15:41 [PATCH v2 0/3] jump_label: get rid of NOP patching where possible Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
@ 2022-06-15 15:41 ` Ard Biesheuvel
  2022-06-26  8:03   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
  2 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2022-06-15 15:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ard Biesheuvel, Peter Zijlstra, Mark Rutland,
	Thomas Bogendoerfer, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	linux-mips, linux-s390

MIPS is the only remaining architecture that needs to patch jump label
NOP encodings to initialize them at load time. So let's move the module
patching part of that from generic code into arch/mips, and drop it from
the others.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/mips/kernel/jump_label.c | 19 ++++++++++++++
 arch/mips/kernel/module.c     |  5 ++--
 arch/s390/kernel/module.c     |  1 -
 arch/sparc/kernel/module.c    |  3 ---
 arch/x86/kernel/module.c      |  3 ---
 include/linux/jump_label.h    |  7 +----
 kernel/jump_label.c           | 27 +-------------------
 7 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/arch/mips/kernel/jump_label.c b/arch/mips/kernel/jump_label.c
index 662c8db9f45b..e4c775e7c063 100644
--- a/arch/mips/kernel/jump_label.c
+++ b/arch/mips/kernel/jump_label.c
@@ -88,3 +88,22 @@ void arch_jump_label_transform(struct jump_entry *e,
 
 	mutex_unlock(&text_mutex);
 }
+
+#ifdef CONFIG_MODULE
+void jump_label_apply_nops(struct module *mod)
+{
+	struct jump_entry *iter_start = mod->jump_entries;
+	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
+	struct jump_entry *iter;
+
+	/* if the module doesn't have jump label entries, just return */
+	if (iter_start == iter_stop)
+		return;
+
+	for (iter = iter_start; iter < iter_stop; iter++) {
+		/* Only write NOPs for arch_branch_static(). */
+		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
+			arch_jump_label_transform(iter, JUMP_LABEL_NOP);
+	}
+}
+#endif
diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 14f46d17500a..0c936cbf20c5 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/jump_label.h>
 
+extern void jump_label_apply_nops(struct module *mod);
 
 struct mips_hi16 {
 	struct mips_hi16 *next;
@@ -428,8 +429,8 @@ int module_finalize(const Elf_Ehdr *hdr,
 	const Elf_Shdr *s;
 	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 
-	/* Make jump label nops. */
-	jump_label_apply_nops(me);
+	if (IS_ENABLED(CONFIG_JUMP_LABEL))
+		jump_label_apply_nops(me);
 
 	INIT_LIST_HEAD(&me->arch.dbe_list);
 	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index 26125a9c436d..2d159b32885b 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -548,6 +548,5 @@ int module_finalize(const Elf_Ehdr *hdr,
 #endif /* CONFIG_FUNCTION_TRACER */
 	}
 
-	jump_label_apply_nops(me);
 	return 0;
 }
diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c
index df39580f398d..66c45a2764bc 100644
--- a/arch/sparc/kernel/module.c
+++ b/arch/sparc/kernel/module.c
@@ -208,9 +208,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 		    const Elf_Shdr *sechdrs,
 		    struct module *me)
 {
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	do_patch_sections(hdr, sechdrs);
 
 	/* Cheetah's I-cache is fully coherent.  */
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index b98ffcf4d250..95b9cf25d4bd 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -304,9 +304,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 					    tseg, tseg + text->sh_size);
 	}
 
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	if (orc && orc_ip)
 		unwind_module_init(me, (void *)orc_ip->sh_addr, orc_ip->sh_size,
 				   (void *)orc->sh_addr, orc->sh_size);
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index bf1eef337a07..2003a0935478 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -230,12 +230,12 @@ extern void static_key_slow_inc(struct static_key *key);
 extern void static_key_slow_dec(struct static_key *key);
 extern void static_key_slow_inc_cpuslocked(struct static_key *key);
 extern void static_key_slow_dec_cpuslocked(struct static_key *key);
-extern void jump_label_apply_nops(struct module *mod);
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
 extern void static_key_enable_cpuslocked(struct static_key *key);
 extern void static_key_disable_cpuslocked(struct static_key *key);
+extern enum jump_label_type jump_label_init_type(struct jump_entry *entry);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -303,11 +303,6 @@ static inline int jump_label_text_reserved(void *start, void *end)
 static inline void jump_label_lock(void) {}
 static inline void jump_label_unlock(void) {}
 
-static inline int jump_label_apply_nops(struct module *mod)
-{
-	return 0;
-}
-
 static inline void static_key_enable(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b156e152d6b4..b1ac2948be79 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -508,7 +508,7 @@ void __init jump_label_init(void)
 
 #ifdef CONFIG_MODULES
 
-static enum jump_label_type jump_label_init_type(struct jump_entry *entry)
+enum jump_label_type jump_label_init_type(struct jump_entry *entry)
 {
 	struct static_key *key = jump_entry_key(entry);
 	bool type = static_key_type(key);
@@ -596,31 +596,6 @@ static void __jump_label_mod_update(struct static_key *key)
 	}
 }
 
-/***
- * apply_jump_label_nops - patch module jump labels with arch_get_jump_label_nop()
- * @mod: module to patch
- *
- * Allow for run-time selection of the optimal nops. Before the module
- * loads patch these with arch_get_jump_label_nop(), which is specified by
- * the arch specific jump label code.
- */
-void jump_label_apply_nops(struct module *mod)
-{
-	struct jump_entry *iter_start = mod->jump_entries;
-	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
-	struct jump_entry *iter;
-
-	/* if the module doesn't have jump label entries, just return */
-	if (iter_start == iter_stop)
-		return;
-
-	for (iter = iter_start; iter < iter_stop; iter++) {
-		/* Only write NOPs for arch_branch_static(). */
-		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
-			arch_jump_label_transform_static(iter, JUMP_LABEL_NOP);
-	}
-}
-
 static int jump_label_add_module(struct module *mod)
 {
 	struct jump_entry *iter_start = mod->jump_entries;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] jump_label: make initial NOP patching the special case
  2022-06-15 15:41 [PATCH v2 0/3] jump_label: get rid of NOP patching where possible Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
  2022-06-15 15:41 ` [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code Ard Biesheuvel
@ 2022-06-15 15:41 ` Ard Biesheuvel
  2022-06-16 11:25   ` Ard Biesheuvel
                     ` (2 more replies)
  2 siblings, 3 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2022-06-15 15:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ard Biesheuvel, Peter Zijlstra, Mark Rutland,
	Thomas Bogendoerfer, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	linux-mips, linux-s390

Instead of defaulting to patching NOP opcodes at init time, and leaving
it to the architectures to override this if this is not needed, switch
to a model where doing nothing is the default. This is the common case
by far, as only MIPS requires NOP patching at init time. On all other
architectures, the correct encodings are emitted by the compiler and so
no initial patching is needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Documentation/staging/static-keys.rst |  3 ---
 arch/arc/kernel/jump_label.c          | 13 -------------
 arch/arm/kernel/jump_label.c          |  6 ------
 arch/arm64/kernel/jump_label.c        | 11 -----------
 arch/mips/include/asm/jump_label.h    |  2 ++
 arch/parisc/kernel/jump_label.c       | 11 -----------
 arch/riscv/kernel/jump_label.c        | 12 ------------
 arch/s390/kernel/jump_label.c         |  5 -----
 arch/x86/kernel/jump_label.c          | 13 -------------
 kernel/jump_label.c                   | 14 +++++---------
 10 files changed, 7 insertions(+), 83 deletions(-)

diff --git a/Documentation/staging/static-keys.rst b/Documentation/staging/static-keys.rst
index 38290b9f25eb..b0a519f456cf 100644
--- a/Documentation/staging/static-keys.rst
+++ b/Documentation/staging/static-keys.rst
@@ -201,9 +201,6 @@ static_key->entry field makes use of the two least significant bits.
 * ``void arch_jump_label_transform(struct jump_entry *entry, enum jump_label_type type)``,
     see: arch/x86/kernel/jump_label.c
 
-* ``__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry, enum jump_label_type type)``,
-    see: arch/x86/kernel/jump_label.c
-
 * ``struct jump_entry``,
     see: arch/x86/include/asm/jump_label.h
 
diff --git a/arch/arc/kernel/jump_label.c b/arch/arc/kernel/jump_label.c
index b8600dc325b5..70b74a5d047b 100644
--- a/arch/arc/kernel/jump_label.c
+++ b/arch/arc/kernel/jump_label.c
@@ -96,19 +96,6 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	flush_icache_range(entry->code, entry->code + JUMP_LABEL_NOP_SIZE);
 }
 
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use only one NOP type (1x, 4 byte) in arch_static_branch, so
-	 * there's no need to patch an identical NOP over the top of it here.
-	 * The generic code calls 'arch_jump_label_transform' if the NOP needs
-	 * to be replaced by a branch, so 'arch_jump_label_transform_static' is
-	 * never called with type other than JUMP_LABEL_NOP.
-	 */
-	BUG_ON(type != JUMP_LABEL_NOP);
-}
-
 #ifdef CONFIG_ARC_DBG_JUMP_LABEL
 #define SELFTEST_MSG	"ARC: instruction generation self-test: "
 
diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
index 303b3ab87f7e..eb9c24b6e8e2 100644
--- a/arch/arm/kernel/jump_label.c
+++ b/arch/arm/kernel/jump_label.c
@@ -27,9 +27,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 {
 	__arch_jump_label_transform(entry, type, false);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	__arch_jump_label_transform(entry, type, true);
-}
diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
index fc98037e1220..faf88ec9c48e 100644
--- a/arch/arm64/kernel/jump_label.c
+++ b/arch/arm64/kernel/jump_label.c
@@ -26,14 +26,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 	aarch64_insn_patch_text_nosync(addr, insn);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the architected A64 NOP in arch_static_branch, so there's no
-	 * need to patch an identical A64 NOP over the top of it here. The core
-	 * will call arch_jump_label_transform from a module notifier if the
-	 * NOP needs to be replaced by a branch.
-	 */
-}
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
index 3185fd3220ec..c5c6864e64bc 100644
--- a/arch/mips/include/asm/jump_label.h
+++ b/arch/mips/include/asm/jump_label.h
@@ -8,6 +8,8 @@
 #ifndef _ASM_MIPS_JUMP_LABEL_H
 #define _ASM_MIPS_JUMP_LABEL_H
 
+#define arch_jump_label_transform_static arch_jump_label_transform
+
 #ifndef __ASSEMBLY__
 
 #include <linux/types.h>
diff --git a/arch/parisc/kernel/jump_label.c b/arch/parisc/kernel/jump_label.c
index d2f3cb12e282..e253b134500d 100644
--- a/arch/parisc/kernel/jump_label.c
+++ b/arch/parisc/kernel/jump_label.c
@@ -42,14 +42,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 	patch_text(addr, insn);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the architected NOP in arch_static_branch, so there's no
-	 * need to patch an identical NOP over the top of it here. The core
-	 * will call arch_jump_label_transform from a module notifier if the
-	 * NOP needs to be replaced by a branch.
-	 */
-}
diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
index 20e09056d141..e6694759dbd0 100644
--- a/arch/riscv/kernel/jump_label.c
+++ b/arch/riscv/kernel/jump_label.c
@@ -39,15 +39,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	patch_text_nosync(addr, &insn, sizeof(insn));
 	mutex_unlock(&text_mutex);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the same instructions in the arch_static_branch and
-	 * arch_static_branch_jump inline functions, so there's no
-	 * need to patch them up here.
-	 * The core will call arch_jump_label_transform  when those
-	 * instructions need to be replaced.
-	 */
-}
diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index d764f0d229ab..e808bb8bc0da 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -80,8 +80,3 @@ void arch_jump_label_transform_apply(void)
 {
 	text_poke_sync();
 }
-
-void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
-						       enum jump_label_type type)
-{
-}
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 68f091ba8443..f5b8ef02d172 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -146,16 +146,3 @@ void arch_jump_label_transform_apply(void)
 	text_poke_finish();
 	mutex_unlock(&text_mutex);
 }
-
-static enum {
-	JL_STATE_START,
-	JL_STATE_NO_UPDATE,
-	JL_STATE_UPDATE,
-} jlstate __initdata_or_module = JL_STATE_START;
-
-__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	if (jlstate == JL_STATE_UPDATE)
-		jump_label_transform(entry, type, 1);
-}
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b1ac2948be79..714ac4c3b556 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -332,17 +332,13 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
 	return 0;
 }
 
-/*
- * Update code which is definitely not currently executing.
- * Architectures which need heavyweight synchronization to modify
- * running code can override this to make the non-live update case
- * cheaper.
- */
-void __weak __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
-					    enum jump_label_type type)
+#ifndef arch_jump_label_transform_static
+static void arch_jump_label_transform_static(struct jump_entry *entry,
+					     enum jump_label_type type)
 {
-	arch_jump_label_transform(entry, type);
+	/* nothing to do on most architectures */
 }
+#endif
 
 static inline struct jump_entry *static_key_entries(struct static_key *key)
 {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] jump_label: make initial NOP patching the special case
  2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
@ 2022-06-16 11:25   ` Ard Biesheuvel
  2022-06-17 13:40     ` Peter Zijlstra
  2022-06-26  8:04   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  2 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2022-06-16 11:25 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: Peter Zijlstra, Mark Rutland, Thomas Bogendoerfer,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, open list:MIPS,
	open list:S390

On Wed, 15 Jun 2022 at 17:41, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Instead of defaulting to patching NOP opcodes at init time, and leaving
> it to the architectures to override this if this is not needed, switch
> to a model where doing nothing is the default. This is the common case
> by far, as only MIPS requires NOP patching at init time. On all other
> architectures, the correct encodings are emitted by the compiler and so
> no initial patching is needed.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  Documentation/staging/static-keys.rst |  3 ---
>  arch/arc/kernel/jump_label.c          | 13 -------------
>  arch/arm/kernel/jump_label.c          |  6 ------
>  arch/arm64/kernel/jump_label.c        | 11 -----------
>  arch/mips/include/asm/jump_label.h    |  2 ++
>  arch/parisc/kernel/jump_label.c       | 11 -----------
>  arch/riscv/kernel/jump_label.c        | 12 ------------
>  arch/s390/kernel/jump_label.c         |  5 -----
>  arch/x86/kernel/jump_label.c          | 13 -------------
>  kernel/jump_label.c                   | 14 +++++---------
>  10 files changed, 7 insertions(+), 83 deletions(-)
>

This needs the following hunk as well, as spotted by the bot:

--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -220,8 +220,6 @@ extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
                                      enum jump_label_type type);
-extern void arch_jump_label_transform_static(struct jump_entry *entry,
-                                            enum jump_label_type type);
 extern bool arch_jump_label_transform_queue(struct jump_entry *entry,
                                            enum jump_label_type type);
 extern void arch_jump_label_transform_apply(void);

Let me know if I need to resend for this.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] jump_label: make initial NOP patching the special case
  2022-06-16 11:25   ` Ard Biesheuvel
@ 2022-06-17 13:40     ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2022-06-17 13:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Kernel Mailing List, Mark Rutland, Thomas Bogendoerfer,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, open list:MIPS,
	open list:S390

On Thu, Jun 16, 2022 at 01:25:02PM +0200, Ard Biesheuvel wrote:
> On Wed, 15 Jun 2022 at 17:41, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > Instead of defaulting to patching NOP opcodes at init time, and leaving
> > it to the architectures to override this if this is not needed, switch
> > to a model where doing nothing is the default. This is the common case
> > by far, as only MIPS requires NOP patching at init time. On all other
> > architectures, the correct encodings are emitted by the compiler and so
> > no initial patching is needed.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  Documentation/staging/static-keys.rst |  3 ---
> >  arch/arc/kernel/jump_label.c          | 13 -------------
> >  arch/arm/kernel/jump_label.c          |  6 ------
> >  arch/arm64/kernel/jump_label.c        | 11 -----------
> >  arch/mips/include/asm/jump_label.h    |  2 ++
> >  arch/parisc/kernel/jump_label.c       | 11 -----------
> >  arch/riscv/kernel/jump_label.c        | 12 ------------
> >  arch/s390/kernel/jump_label.c         |  5 -----
> >  arch/x86/kernel/jump_label.c          | 13 -------------
> >  kernel/jump_label.c                   | 14 +++++---------
> >  10 files changed, 7 insertions(+), 83 deletions(-)
> >
> 
> This needs the following hunk as well, as spotted by the bot:
> 
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -220,8 +220,6 @@ extern void jump_label_lock(void);
>  extern void jump_label_unlock(void);
>  extern void arch_jump_label_transform(struct jump_entry *entry,
>                                       enum jump_label_type type);
> -extern void arch_jump_label_transform_static(struct jump_entry *entry,
> -                                            enum jump_label_type type);
>  extern bool arch_jump_label_transform_queue(struct jump_entry *entry,
>                                             enum jump_label_type type);
>  extern void arch_jump_label_transform_apply(void);
> 
Done, Thanks!




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching
  2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
@ 2022-06-26  7:57   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: Alexander Gordeev @ 2022-06-26  7:57 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-kernel, Peter Zijlstra, Mark Rutland, Thomas Bogendoerfer,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	Sven Schnelle, linux-mips, linux-s390

On Wed, Jun 15, 2022 at 05:41:40PM +0200, Ard Biesheuvel wrote:
> Patching NOPs into other NOPs at boot time serves no purpose, so let's
> use the same NOP encodings at compile time and runtime.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/s390/include/asm/jump_label.h |  5 ++---
>  arch/s390/kernel/jump_label.c      | 23 +++++---------------
>  2 files changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
> index 916cfcb36d8a..895f774bbcc5 100644
> --- a/arch/s390/include/asm/jump_label.h
> +++ b/arch/s390/include/asm/jump_label.h
> @@ -10,7 +10,6 @@
>  #include <linux/stringify.h>
>  
>  #define JUMP_LABEL_NOP_SIZE 6
> -#define JUMP_LABEL_NOP_OFFSET 2
>  
>  #ifdef CONFIG_CC_IS_CLANG
>  #define JUMP_LABEL_STATIC_KEY_CONSTRAINT "i"
> @@ -21,12 +20,12 @@
>  #endif
>  
>  /*
> - * We use a brcl 0,2 instruction for jump labels at compile time so it
> + * We use a brcl 0,<offset> instruction for jump labels so it
>   * can be easily distinguished from a hotpatch generated instruction.
>   */
>  static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
>  {
> -	asm_volatile_goto("0:	brcl	0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n"
> +	asm_volatile_goto("0:	brcl 0,%l[label]\n"

Please, use tab after brcl, not space.

>  			  ".pushsection __jump_table,\"aw\"\n"
>  			  ".balign	8\n"
>  			  ".long	0b-.,%l[label]-.\n"
> diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
> index 6bec000c6c1c..d764f0d229ab 100644
> --- a/arch/s390/kernel/jump_label.c
> +++ b/arch/s390/kernel/jump_label.c
> @@ -44,14 +44,8 @@ static void jump_label_bug(struct jump_entry *entry, struct insn *expected,
>  	panic("Corrupted kernel text");
>  }
>  
> -static struct insn orignop = {
> -	.opcode = 0xc004,
> -	.offset = JUMP_LABEL_NOP_OFFSET >> 1,
> -};
> -
>  static void jump_label_transform(struct jump_entry *entry,
> -				 enum jump_label_type type,
> -				 int init)
> +				 enum jump_label_type type)
>  {
>  	void *code = (void *)jump_entry_code(entry);
>  	struct insn old, new;
> @@ -63,27 +57,22 @@ static void jump_label_transform(struct jump_entry *entry,
>  		jump_label_make_branch(entry, &old);
>  		jump_label_make_nop(entry, &new);
>  	}
> -	if (init) {
> -		if (memcmp(code, &orignop, sizeof(orignop)))
> -			jump_label_bug(entry, &orignop, &new);
> -	} else {
> -		if (memcmp(code, &old, sizeof(old)))
> -			jump_label_bug(entry, &old, &new);
> -	}
> +	if (memcmp(code, &old, sizeof(old)))
> +		jump_label_bug(entry, &old, &new);
>  	s390_kernel_write(code, &new, sizeof(new));
>  }
>  
>  void arch_jump_label_transform(struct jump_entry *entry,
>  			       enum jump_label_type type)
>  {
> -	jump_label_transform(entry, type, 0);
> +	jump_label_transform(entry, type);
>  	text_poke_sync();
>  }
>  
>  bool arch_jump_label_transform_queue(struct jump_entry *entry,
>  				     enum jump_label_type type)
>  {
> -	jump_label_transform(entry, type, 0);
> +	jump_label_transform(entry, type);
>  	return true;
>  }
>  
> @@ -95,6 +84,4 @@ void arch_jump_label_transform_apply(void)
>  void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
>  						       enum jump_label_type type)
>  {
> -	jump_label_transform(entry, type, 1);
> -	text_poke_sync();
>  }


With the comment above:

Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>

Thanks!

> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code
  2022-06-15 15:41 ` [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code Ard Biesheuvel
@ 2022-06-26  8:03   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: Alexander Gordeev @ 2022-06-26  8:03 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-kernel, Peter Zijlstra, Mark Rutland, Thomas Bogendoerfer,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	Sven Schnelle, linux-mips, linux-s390

On Wed, Jun 15, 2022 at 05:41:41PM +0200, Ard Biesheuvel wrote:
> MIPS is the only remaining architecture that needs to patch jump label
> NOP encodings to initialize them at load time. So let's move the module
> patching part of that from generic code into arch/mips, and drop it from
> the others.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/mips/kernel/jump_label.c | 19 ++++++++++++++
>  arch/mips/kernel/module.c     |  5 ++--
>  arch/s390/kernel/module.c     |  1 -
>  arch/sparc/kernel/module.c    |  3 ---
>  arch/x86/kernel/module.c      |  3 ---
>  include/linux/jump_label.h    |  7 +----
>  kernel/jump_label.c           | 27 +-------------------
>  7 files changed, 24 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/mips/kernel/jump_label.c b/arch/mips/kernel/jump_label.c
> index 662c8db9f45b..e4c775e7c063 100644
> --- a/arch/mips/kernel/jump_label.c
> +++ b/arch/mips/kernel/jump_label.c
> @@ -88,3 +88,22 @@ void arch_jump_label_transform(struct jump_entry *e,
>  
>  	mutex_unlock(&text_mutex);
>  }
> +
> +#ifdef CONFIG_MODULE
> +void jump_label_apply_nops(struct module *mod)
> +{
> +	struct jump_entry *iter_start = mod->jump_entries;
> +	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
> +	struct jump_entry *iter;
> +
> +	/* if the module doesn't have jump label entries, just return */
> +	if (iter_start == iter_stop)
> +		return;
> +
> +	for (iter = iter_start; iter < iter_stop; iter++) {
> +		/* Only write NOPs for arch_branch_static(). */
> +		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
> +			arch_jump_label_transform(iter, JUMP_LABEL_NOP);
> +	}
> +}
> +#endif
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index 14f46d17500a..0c936cbf20c5 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -21,6 +21,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/jump_label.h>
>  
> +extern void jump_label_apply_nops(struct module *mod);
>  
>  struct mips_hi16 {
>  	struct mips_hi16 *next;
> @@ -428,8 +429,8 @@ int module_finalize(const Elf_Ehdr *hdr,
>  	const Elf_Shdr *s;
>  	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  
> -	/* Make jump label nops. */
> -	jump_label_apply_nops(me);
> +	if (IS_ENABLED(CONFIG_JUMP_LABEL))
> +		jump_label_apply_nops(me);
>  
>  	INIT_LIST_HEAD(&me->arch.dbe_list);
>  	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
> diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> index 26125a9c436d..2d159b32885b 100644
> --- a/arch/s390/kernel/module.c
> +++ b/arch/s390/kernel/module.c
> @@ -548,6 +548,5 @@ int module_finalize(const Elf_Ehdr *hdr,
>  #endif /* CONFIG_FUNCTION_TRACER */
>  	}
>  
> -	jump_label_apply_nops(me);
>  	return 0;
>  }
> diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c
> index df39580f398d..66c45a2764bc 100644
> --- a/arch/sparc/kernel/module.c
> +++ b/arch/sparc/kernel/module.c
> @@ -208,9 +208,6 @@ int module_finalize(const Elf_Ehdr *hdr,
>  		    const Elf_Shdr *sechdrs,
>  		    struct module *me)
>  {
> -	/* make jump label nops */
> -	jump_label_apply_nops(me);
> -
>  	do_patch_sections(hdr, sechdrs);
>  
>  	/* Cheetah's I-cache is fully coherent.  */
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index b98ffcf4d250..95b9cf25d4bd 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -304,9 +304,6 @@ int module_finalize(const Elf_Ehdr *hdr,
>  					    tseg, tseg + text->sh_size);
>  	}
>  
> -	/* make jump label nops */
> -	jump_label_apply_nops(me);
> -
>  	if (orc && orc_ip)
>  		unwind_module_init(me, (void *)orc_ip->sh_addr, orc_ip->sh_size,
>  				   (void *)orc->sh_addr, orc->sh_size);
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index bf1eef337a07..2003a0935478 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -230,12 +230,12 @@ extern void static_key_slow_inc(struct static_key *key);
>  extern void static_key_slow_dec(struct static_key *key);
>  extern void static_key_slow_inc_cpuslocked(struct static_key *key);
>  extern void static_key_slow_dec_cpuslocked(struct static_key *key);
> -extern void jump_label_apply_nops(struct module *mod);
>  extern int static_key_count(struct static_key *key);
>  extern void static_key_enable(struct static_key *key);
>  extern void static_key_disable(struct static_key *key);
>  extern void static_key_enable_cpuslocked(struct static_key *key);
>  extern void static_key_disable_cpuslocked(struct static_key *key);
> +extern enum jump_label_type jump_label_init_type(struct jump_entry *entry);
>  
>  /*
>   * We should be using ATOMIC_INIT() for initializing .enabled, but
> @@ -303,11 +303,6 @@ static inline int jump_label_text_reserved(void *start, void *end)
>  static inline void jump_label_lock(void) {}
>  static inline void jump_label_unlock(void) {}
>  
> -static inline int jump_label_apply_nops(struct module *mod)
> -{
> -	return 0;
> -}
> -
>  static inline void static_key_enable(struct static_key *key)
>  {
>  	STATIC_KEY_CHECK_USE(key);
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index b156e152d6b4..b1ac2948be79 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -508,7 +508,7 @@ void __init jump_label_init(void)
>  
>  #ifdef CONFIG_MODULES
>  
> -static enum jump_label_type jump_label_init_type(struct jump_entry *entry)
> +enum jump_label_type jump_label_init_type(struct jump_entry *entry)
>  {
>  	struct static_key *key = jump_entry_key(entry);
>  	bool type = static_key_type(key);
> @@ -596,31 +596,6 @@ static void __jump_label_mod_update(struct static_key *key)
>  	}
>  }
>  
> -/***
> - * apply_jump_label_nops - patch module jump labels with arch_get_jump_label_nop()
> - * @mod: module to patch
> - *
> - * Allow for run-time selection of the optimal nops. Before the module
> - * loads patch these with arch_get_jump_label_nop(), which is specified by
> - * the arch specific jump label code.
> - */
> -void jump_label_apply_nops(struct module *mod)
> -{
> -	struct jump_entry *iter_start = mod->jump_entries;
> -	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
> -	struct jump_entry *iter;
> -
> -	/* if the module doesn't have jump label entries, just return */
> -	if (iter_start == iter_stop)
> -		return;
> -
> -	for (iter = iter_start; iter < iter_stop; iter++) {
> -		/* Only write NOPs for arch_branch_static(). */
> -		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
> -			arch_jump_label_transform_static(iter, JUMP_LABEL_NOP);
> -	}
> -}
> -
>  static int jump_label_add_module(struct module *mod)
>  {
>  	struct jump_entry *iter_start = mod->jump_entries;

To the common and s390 parts:

Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>

FWIW, the change is not mips-only, thus Subject is not exactly correct.

> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] jump_label: make initial NOP patching the special case
  2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
  2022-06-16 11:25   ` Ard Biesheuvel
@ 2022-06-26  8:04   ` Alexander Gordeev
  2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
  2 siblings, 0 replies; 12+ messages in thread
From: Alexander Gordeev @ 2022-06-26  8:04 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-kernel, Peter Zijlstra, Mark Rutland, Thomas Bogendoerfer,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	Sven Schnelle, linux-mips, linux-s390

On Wed, Jun 15, 2022 at 05:41:42PM +0200, Ard Biesheuvel wrote:
> Instead of defaulting to patching NOP opcodes at init time, and leaving
> it to the architectures to override this if this is not needed, switch
> to a model where doing nothing is the default. This is the common case
> by far, as only MIPS requires NOP patching at init time. On all other
> architectures, the correct encodings are emitted by the compiler and so
> no initial patching is needed.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  Documentation/staging/static-keys.rst |  3 ---
>  arch/arc/kernel/jump_label.c          | 13 -------------
>  arch/arm/kernel/jump_label.c          |  6 ------
>  arch/arm64/kernel/jump_label.c        | 11 -----------
>  arch/mips/include/asm/jump_label.h    |  2 ++
>  arch/parisc/kernel/jump_label.c       | 11 -----------
>  arch/riscv/kernel/jump_label.c        | 12 ------------
>  arch/s390/kernel/jump_label.c         |  5 -----
>  arch/x86/kernel/jump_label.c          | 13 -------------
>  kernel/jump_label.c                   | 14 +++++---------
>  10 files changed, 7 insertions(+), 83 deletions(-)
> 
> diff --git a/Documentation/staging/static-keys.rst b/Documentation/staging/static-keys.rst
> index 38290b9f25eb..b0a519f456cf 100644
> --- a/Documentation/staging/static-keys.rst
> +++ b/Documentation/staging/static-keys.rst
> @@ -201,9 +201,6 @@ static_key->entry field makes use of the two least significant bits.
>  * ``void arch_jump_label_transform(struct jump_entry *entry, enum jump_label_type type)``,
>      see: arch/x86/kernel/jump_label.c
>  
> -* ``__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry, enum jump_label_type type)``,
> -    see: arch/x86/kernel/jump_label.c
> -
>  * ``struct jump_entry``,
>      see: arch/x86/include/asm/jump_label.h
>  
> diff --git a/arch/arc/kernel/jump_label.c b/arch/arc/kernel/jump_label.c
> index b8600dc325b5..70b74a5d047b 100644
> --- a/arch/arc/kernel/jump_label.c
> +++ b/arch/arc/kernel/jump_label.c
> @@ -96,19 +96,6 @@ void arch_jump_label_transform(struct jump_entry *entry,
>  	flush_icache_range(entry->code, entry->code + JUMP_LABEL_NOP_SIZE);
>  }
>  
> -void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	/*
> -	 * We use only one NOP type (1x, 4 byte) in arch_static_branch, so
> -	 * there's no need to patch an identical NOP over the top of it here.
> -	 * The generic code calls 'arch_jump_label_transform' if the NOP needs
> -	 * to be replaced by a branch, so 'arch_jump_label_transform_static' is
> -	 * never called with type other than JUMP_LABEL_NOP.
> -	 */
> -	BUG_ON(type != JUMP_LABEL_NOP);
> -}
> -
>  #ifdef CONFIG_ARC_DBG_JUMP_LABEL
>  #define SELFTEST_MSG	"ARC: instruction generation self-test: "
>  
> diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
> index 303b3ab87f7e..eb9c24b6e8e2 100644
> --- a/arch/arm/kernel/jump_label.c
> +++ b/arch/arm/kernel/jump_label.c
> @@ -27,9 +27,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
>  {
>  	__arch_jump_label_transform(entry, type, false);
>  }
> -
> -void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	__arch_jump_label_transform(entry, type, true);
> -}
> diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
> index fc98037e1220..faf88ec9c48e 100644
> --- a/arch/arm64/kernel/jump_label.c
> +++ b/arch/arm64/kernel/jump_label.c
> @@ -26,14 +26,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
>  
>  	aarch64_insn_patch_text_nosync(addr, insn);
>  }
> -
> -void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	/*
> -	 * We use the architected A64 NOP in arch_static_branch, so there's no
> -	 * need to patch an identical A64 NOP over the top of it here. The core
> -	 * will call arch_jump_label_transform from a module notifier if the
> -	 * NOP needs to be replaced by a branch.
> -	 */
> -}
> diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
> index 3185fd3220ec..c5c6864e64bc 100644
> --- a/arch/mips/include/asm/jump_label.h
> +++ b/arch/mips/include/asm/jump_label.h
> @@ -8,6 +8,8 @@
>  #ifndef _ASM_MIPS_JUMP_LABEL_H
>  #define _ASM_MIPS_JUMP_LABEL_H
>  
> +#define arch_jump_label_transform_static arch_jump_label_transform
> +
>  #ifndef __ASSEMBLY__
>  
>  #include <linux/types.h>
> diff --git a/arch/parisc/kernel/jump_label.c b/arch/parisc/kernel/jump_label.c
> index d2f3cb12e282..e253b134500d 100644
> --- a/arch/parisc/kernel/jump_label.c
> +++ b/arch/parisc/kernel/jump_label.c
> @@ -42,14 +42,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
>  
>  	patch_text(addr, insn);
>  }
> -
> -void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	/*
> -	 * We use the architected NOP in arch_static_branch, so there's no
> -	 * need to patch an identical NOP over the top of it here. The core
> -	 * will call arch_jump_label_transform from a module notifier if the
> -	 * NOP needs to be replaced by a branch.
> -	 */
> -}
> diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
> index 20e09056d141..e6694759dbd0 100644
> --- a/arch/riscv/kernel/jump_label.c
> +++ b/arch/riscv/kernel/jump_label.c
> @@ -39,15 +39,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
>  	patch_text_nosync(addr, &insn, sizeof(insn));
>  	mutex_unlock(&text_mutex);
>  }
> -
> -void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	/*
> -	 * We use the same instructions in the arch_static_branch and
> -	 * arch_static_branch_jump inline functions, so there's no
> -	 * need to patch them up here.
> -	 * The core will call arch_jump_label_transform  when those
> -	 * instructions need to be replaced.
> -	 */
> -}
> diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
> index d764f0d229ab..e808bb8bc0da 100644
> --- a/arch/s390/kernel/jump_label.c
> +++ b/arch/s390/kernel/jump_label.c
> @@ -80,8 +80,3 @@ void arch_jump_label_transform_apply(void)
>  {
>  	text_poke_sync();
>  }
> -
> -void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
> -						       enum jump_label_type type)
> -{
> -}
> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> index 68f091ba8443..f5b8ef02d172 100644
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -146,16 +146,3 @@ void arch_jump_label_transform_apply(void)
>  	text_poke_finish();
>  	mutex_unlock(&text_mutex);
>  }
> -
> -static enum {
> -	JL_STATE_START,
> -	JL_STATE_NO_UPDATE,
> -	JL_STATE_UPDATE,
> -} jlstate __initdata_or_module = JL_STATE_START;
> -
> -__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
> -				      enum jump_label_type type)
> -{
> -	if (jlstate == JL_STATE_UPDATE)
> -		jump_label_transform(entry, type, 1);
> -}
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index b1ac2948be79..714ac4c3b556 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -332,17 +332,13 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
>  	return 0;
>  }
>  
> -/*
> - * Update code which is definitely not currently executing.
> - * Architectures which need heavyweight synchronization to modify
> - * running code can override this to make the non-live update case
> - * cheaper.
> - */
> -void __weak __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
> -					    enum jump_label_type type)
> +#ifndef arch_jump_label_transform_static
> +static void arch_jump_label_transform_static(struct jump_entry *entry,
> +					     enum jump_label_type type)
>  {
> -	arch_jump_label_transform(entry, type);
> +	/* nothing to do on most architectures */
>  }
> +#endif
>  
>  static inline struct jump_entry *static_key_entries(struct static_key *key)
>  {

With the follow-up fixup to the common and s390 parts:

Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>

> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip: locking/core] jump_label: make initial NOP patching the special case
  2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
  2022-06-16 11:25   ` Ard Biesheuvel
  2022-06-26  8:04   ` Alexander Gordeev
@ 2022-06-28  7:09   ` tip-bot2 for Ard Biesheuvel
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Ard Biesheuvel @ 2022-06-28  7:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ard Biesheuvel, Peter Zijlstra (Intel), Mark Rutland, x86, linux-kernel

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

Commit-ID:     7e6b9db27de9f69a705c1a046d45882c768e16c3
Gitweb:        https://git.kernel.org/tip/7e6b9db27de9f69a705c1a046d45882c768e16c3
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Wed, 15 Jun 2022 17:41:42 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 24 Jun 2022 09:48:55 +02:00

jump_label: make initial NOP patching the special case

Instead of defaulting to patching NOP opcodes at init time, and leaving
it to the architectures to override this if this is not needed, switch
to a model where doing nothing is the default. This is the common case
by far, as only MIPS requires NOP patching at init time. On all other
architectures, the correct encodings are emitted by the compiler and so
no initial patching is needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220615154142.1574619-4-ardb@kernel.org
---
 Documentation/staging/static-keys.rst |  3 ---
 arch/arc/kernel/jump_label.c          | 13 -------------
 arch/arm/kernel/jump_label.c          |  6 ------
 arch/arm64/kernel/jump_label.c        | 11 -----------
 arch/mips/include/asm/jump_label.h    |  2 ++
 arch/parisc/kernel/jump_label.c       | 11 -----------
 arch/riscv/kernel/jump_label.c        | 12 ------------
 arch/s390/kernel/jump_label.c         |  5 -----
 arch/x86/kernel/jump_label.c          | 13 -------------
 include/linux/jump_label.h            |  2 --
 kernel/jump_label.c                   | 14 +++++---------
 11 files changed, 7 insertions(+), 85 deletions(-)

diff --git a/Documentation/staging/static-keys.rst b/Documentation/staging/static-keys.rst
index 38290b9..b0a519f 100644
--- a/Documentation/staging/static-keys.rst
+++ b/Documentation/staging/static-keys.rst
@@ -201,9 +201,6 @@ static_key->entry field makes use of the two least significant bits.
 * ``void arch_jump_label_transform(struct jump_entry *entry, enum jump_label_type type)``,
     see: arch/x86/kernel/jump_label.c
 
-* ``__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry, enum jump_label_type type)``,
-    see: arch/x86/kernel/jump_label.c
-
 * ``struct jump_entry``,
     see: arch/x86/include/asm/jump_label.h
 
diff --git a/arch/arc/kernel/jump_label.c b/arch/arc/kernel/jump_label.c
index b8600dc..70b74a5 100644
--- a/arch/arc/kernel/jump_label.c
+++ b/arch/arc/kernel/jump_label.c
@@ -96,19 +96,6 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	flush_icache_range(entry->code, entry->code + JUMP_LABEL_NOP_SIZE);
 }
 
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use only one NOP type (1x, 4 byte) in arch_static_branch, so
-	 * there's no need to patch an identical NOP over the top of it here.
-	 * The generic code calls 'arch_jump_label_transform' if the NOP needs
-	 * to be replaced by a branch, so 'arch_jump_label_transform_static' is
-	 * never called with type other than JUMP_LABEL_NOP.
-	 */
-	BUG_ON(type != JUMP_LABEL_NOP);
-}
-
 #ifdef CONFIG_ARC_DBG_JUMP_LABEL
 #define SELFTEST_MSG	"ARC: instruction generation self-test: "
 
diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
index 303b3ab..eb9c24b 100644
--- a/arch/arm/kernel/jump_label.c
+++ b/arch/arm/kernel/jump_label.c
@@ -27,9 +27,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 {
 	__arch_jump_label_transform(entry, type, false);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	__arch_jump_label_transform(entry, type, true);
-}
diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
index fc98037..faf88ec 100644
--- a/arch/arm64/kernel/jump_label.c
+++ b/arch/arm64/kernel/jump_label.c
@@ -26,14 +26,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 	aarch64_insn_patch_text_nosync(addr, insn);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the architected A64 NOP in arch_static_branch, so there's no
-	 * need to patch an identical A64 NOP over the top of it here. The core
-	 * will call arch_jump_label_transform from a module notifier if the
-	 * NOP needs to be replaced by a branch.
-	 */
-}
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
index 3185fd3..c5c6864 100644
--- a/arch/mips/include/asm/jump_label.h
+++ b/arch/mips/include/asm/jump_label.h
@@ -8,6 +8,8 @@
 #ifndef _ASM_MIPS_JUMP_LABEL_H
 #define _ASM_MIPS_JUMP_LABEL_H
 
+#define arch_jump_label_transform_static arch_jump_label_transform
+
 #ifndef __ASSEMBLY__
 
 #include <linux/types.h>
diff --git a/arch/parisc/kernel/jump_label.c b/arch/parisc/kernel/jump_label.c
index d2f3cb1..e253b13 100644
--- a/arch/parisc/kernel/jump_label.c
+++ b/arch/parisc/kernel/jump_label.c
@@ -42,14 +42,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 
 	patch_text(addr, insn);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the architected NOP in arch_static_branch, so there's no
-	 * need to patch an identical NOP over the top of it here. The core
-	 * will call arch_jump_label_transform from a module notifier if the
-	 * NOP needs to be replaced by a branch.
-	 */
-}
diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
index 20e0905..e669475 100644
--- a/arch/riscv/kernel/jump_label.c
+++ b/arch/riscv/kernel/jump_label.c
@@ -39,15 +39,3 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	patch_text_nosync(addr, &insn, sizeof(insn));
 	mutex_unlock(&text_mutex);
 }
-
-void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	/*
-	 * We use the same instructions in the arch_static_branch and
-	 * arch_static_branch_jump inline functions, so there's no
-	 * need to patch them up here.
-	 * The core will call arch_jump_label_transform  when those
-	 * instructions need to be replaced.
-	 */
-}
diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index d764f0d..e808bb8 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -80,8 +80,3 @@ void arch_jump_label_transform_apply(void)
 {
 	text_poke_sync();
 }
-
-void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
-						       enum jump_label_type type)
-{
-}
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 68f091b..f5b8ef0 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -146,16 +146,3 @@ void arch_jump_label_transform_apply(void)
 	text_poke_finish();
 	mutex_unlock(&text_mutex);
 }
-
-static enum {
-	JL_STATE_START,
-	JL_STATE_NO_UPDATE,
-	JL_STATE_UPDATE,
-} jlstate __initdata_or_module = JL_STATE_START;
-
-__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
-				      enum jump_label_type type)
-{
-	if (jlstate == JL_STATE_UPDATE)
-		jump_label_transform(entry, type, 1);
-}
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 2003a09..570831c 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -220,8 +220,6 @@ extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
 				      enum jump_label_type type);
-extern void arch_jump_label_transform_static(struct jump_entry *entry,
-					     enum jump_label_type type);
 extern bool arch_jump_label_transform_queue(struct jump_entry *entry,
 					    enum jump_label_type type);
 extern void arch_jump_label_transform_apply(void);
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b1ac294..714ac4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -332,17 +332,13 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
 	return 0;
 }
 
-/*
- * Update code which is definitely not currently executing.
- * Architectures which need heavyweight synchronization to modify
- * running code can override this to make the non-live update case
- * cheaper.
- */
-void __weak __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
-					    enum jump_label_type type)
+#ifndef arch_jump_label_transform_static
+static void arch_jump_label_transform_static(struct jump_entry *entry,
+					     enum jump_label_type type)
 {
-	arch_jump_label_transform(entry, type);
+	/* nothing to do on most architectures */
 }
+#endif
 
 static inline struct jump_entry *static_key_entries(struct static_key *key)
 {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip: locking/core] jump_label: mips: move module NOP patching into arch code
  2022-06-15 15:41 ` [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code Ard Biesheuvel
  2022-06-26  8:03   ` Alexander Gordeev
@ 2022-06-28  7:09   ` tip-bot2 for Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot2 for Ard Biesheuvel @ 2022-06-28  7:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ard Biesheuvel, Peter Zijlstra (Intel), x86, linux-kernel

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

Commit-ID:     fdfd42892f311e2b3695852036e5be23661dc590
Gitweb:        https://git.kernel.org/tip/fdfd42892f311e2b3695852036e5be23661dc590
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Wed, 15 Jun 2022 17:41:41 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 24 Jun 2022 09:48:55 +02:00

jump_label: mips: move module NOP patching into arch code

MIPS is the only remaining architecture that needs to patch jump label
NOP encodings to initialize them at load time. So let's move the module
patching part of that from generic code into arch/mips, and drop it from
the others.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220615154142.1574619-3-ardb@kernel.org
---
 arch/mips/kernel/jump_label.c | 19 +++++++++++++++++++
 arch/mips/kernel/module.c     |  5 +++--
 arch/s390/kernel/module.c     |  1 -
 arch/sparc/kernel/module.c    |  3 ---
 arch/x86/kernel/module.c      |  3 ---
 include/linux/jump_label.h    |  7 +------
 kernel/jump_label.c           | 27 +--------------------------
 7 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/arch/mips/kernel/jump_label.c b/arch/mips/kernel/jump_label.c
index 662c8db..71a882c 100644
--- a/arch/mips/kernel/jump_label.c
+++ b/arch/mips/kernel/jump_label.c
@@ -88,3 +88,22 @@ void arch_jump_label_transform(struct jump_entry *e,
 
 	mutex_unlock(&text_mutex);
 }
+
+#ifdef CONFIG_MODULES
+void jump_label_apply_nops(struct module *mod)
+{
+	struct jump_entry *iter_start = mod->jump_entries;
+	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
+	struct jump_entry *iter;
+
+	/* if the module doesn't have jump label entries, just return */
+	if (iter_start == iter_stop)
+		return;
+
+	for (iter = iter_start; iter < iter_stop; iter++) {
+		/* Only write NOPs for arch_branch_static(). */
+		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
+			arch_jump_label_transform(iter, JUMP_LABEL_NOP);
+	}
+}
+#endif
diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 14f46d1..0c936cb 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/jump_label.h>
 
+extern void jump_label_apply_nops(struct module *mod);
 
 struct mips_hi16 {
 	struct mips_hi16 *next;
@@ -428,8 +429,8 @@ int module_finalize(const Elf_Ehdr *hdr,
 	const Elf_Shdr *s;
 	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 
-	/* Make jump label nops. */
-	jump_label_apply_nops(me);
+	if (IS_ENABLED(CONFIG_JUMP_LABEL))
+		jump_label_apply_nops(me);
 
 	INIT_LIST_HEAD(&me->arch.dbe_list);
 	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index 26125a9..2d159b3 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -548,6 +548,5 @@ int module_finalize(const Elf_Ehdr *hdr,
 #endif /* CONFIG_FUNCTION_TRACER */
 	}
 
-	jump_label_apply_nops(me);
 	return 0;
 }
diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c
index df39580..66c45a2 100644
--- a/arch/sparc/kernel/module.c
+++ b/arch/sparc/kernel/module.c
@@ -208,9 +208,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 		    const Elf_Shdr *sechdrs,
 		    struct module *me)
 {
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	do_patch_sections(hdr, sechdrs);
 
 	/* Cheetah's I-cache is fully coherent.  */
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index b98ffcf..95b9cf2 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -304,9 +304,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 					    tseg, tseg + text->sh_size);
 	}
 
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	if (orc && orc_ip)
 		unwind_module_init(me, (void *)orc_ip->sh_addr, orc_ip->sh_size,
 				   (void *)orc->sh_addr, orc->sh_size);
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index bf1eef3..2003a09 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -230,12 +230,12 @@ extern void static_key_slow_inc(struct static_key *key);
 extern void static_key_slow_dec(struct static_key *key);
 extern void static_key_slow_inc_cpuslocked(struct static_key *key);
 extern void static_key_slow_dec_cpuslocked(struct static_key *key);
-extern void jump_label_apply_nops(struct module *mod);
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
 extern void static_key_enable_cpuslocked(struct static_key *key);
 extern void static_key_disable_cpuslocked(struct static_key *key);
+extern enum jump_label_type jump_label_init_type(struct jump_entry *entry);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -303,11 +303,6 @@ static inline int jump_label_text_reserved(void *start, void *end)
 static inline void jump_label_lock(void) {}
 static inline void jump_label_unlock(void) {}
 
-static inline int jump_label_apply_nops(struct module *mod)
-{
-	return 0;
-}
-
 static inline void static_key_enable(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b156e15..b1ac294 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -508,7 +508,7 @@ void __init jump_label_init(void)
 
 #ifdef CONFIG_MODULES
 
-static enum jump_label_type jump_label_init_type(struct jump_entry *entry)
+enum jump_label_type jump_label_init_type(struct jump_entry *entry)
 {
 	struct static_key *key = jump_entry_key(entry);
 	bool type = static_key_type(key);
@@ -596,31 +596,6 @@ static void __jump_label_mod_update(struct static_key *key)
 	}
 }
 
-/***
- * apply_jump_label_nops - patch module jump labels with arch_get_jump_label_nop()
- * @mod: module to patch
- *
- * Allow for run-time selection of the optimal nops. Before the module
- * loads patch these with arch_get_jump_label_nop(), which is specified by
- * the arch specific jump label code.
- */
-void jump_label_apply_nops(struct module *mod)
-{
-	struct jump_entry *iter_start = mod->jump_entries;
-	struct jump_entry *iter_stop = iter_start + mod->num_jump_entries;
-	struct jump_entry *iter;
-
-	/* if the module doesn't have jump label entries, just return */
-	if (iter_start == iter_stop)
-		return;
-
-	for (iter = iter_start; iter < iter_stop; iter++) {
-		/* Only write NOPs for arch_branch_static(). */
-		if (jump_label_init_type(iter) == JUMP_LABEL_NOP)
-			arch_jump_label_transform_static(iter, JUMP_LABEL_NOP);
-	}
-}
-
 static int jump_label_add_module(struct module *mod)
 {
 	struct jump_entry *iter_start = mod->jump_entries;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip: locking/core] jump_label: s390: avoid pointless initial NOP patching
  2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
  2022-06-26  7:57   ` Alexander Gordeev
@ 2022-06-28  7:09   ` tip-bot2 for Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot2 for Ard Biesheuvel @ 2022-06-28  7:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ard Biesheuvel, Peter Zijlstra (Intel), x86, linux-kernel

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

Commit-ID:     0c3b61e00a0d0872c521586494ec23f6016c317a
Gitweb:        https://git.kernel.org/tip/0c3b61e00a0d0872c521586494ec23f6016c317a
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Wed, 15 Jun 2022 17:41:40 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 24 Jun 2022 09:48:54 +02:00

jump_label: s390: avoid pointless initial NOP patching

Patching NOPs into other NOPs at boot time serves no purpose, so let's
use the same NOP encodings at compile time and runtime.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220615154142.1574619-2-ardb@kernel.org
---
 arch/s390/include/asm/jump_label.h |  5 ++---
 arch/s390/kernel/jump_label.c      | 23 +++++------------------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
index 916cfcb..895f774 100644
--- a/arch/s390/include/asm/jump_label.h
+++ b/arch/s390/include/asm/jump_label.h
@@ -10,7 +10,6 @@
 #include <linux/stringify.h>
 
 #define JUMP_LABEL_NOP_SIZE 6
-#define JUMP_LABEL_NOP_OFFSET 2
 
 #ifdef CONFIG_CC_IS_CLANG
 #define JUMP_LABEL_STATIC_KEY_CONSTRAINT "i"
@@ -21,12 +20,12 @@
 #endif
 
 /*
- * We use a brcl 0,2 instruction for jump labels at compile time so it
+ * We use a brcl 0,<offset> instruction for jump labels so it
  * can be easily distinguished from a hotpatch generated instruction.
  */
 static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
 {
-	asm_volatile_goto("0:	brcl	0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n"
+	asm_volatile_goto("0:	brcl 0,%l[label]\n"
 			  ".pushsection __jump_table,\"aw\"\n"
 			  ".balign	8\n"
 			  ".long	0b-.,%l[label]-.\n"
diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index 6bec000..d764f0d 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -44,14 +44,8 @@ static void jump_label_bug(struct jump_entry *entry, struct insn *expected,
 	panic("Corrupted kernel text");
 }
 
-static struct insn orignop = {
-	.opcode = 0xc004,
-	.offset = JUMP_LABEL_NOP_OFFSET >> 1,
-};
-
 static void jump_label_transform(struct jump_entry *entry,
-				 enum jump_label_type type,
-				 int init)
+				 enum jump_label_type type)
 {
 	void *code = (void *)jump_entry_code(entry);
 	struct insn old, new;
@@ -63,27 +57,22 @@ static void jump_label_transform(struct jump_entry *entry,
 		jump_label_make_branch(entry, &old);
 		jump_label_make_nop(entry, &new);
 	}
-	if (init) {
-		if (memcmp(code, &orignop, sizeof(orignop)))
-			jump_label_bug(entry, &orignop, &new);
-	} else {
-		if (memcmp(code, &old, sizeof(old)))
-			jump_label_bug(entry, &old, &new);
-	}
+	if (memcmp(code, &old, sizeof(old)))
+		jump_label_bug(entry, &old, &new);
 	s390_kernel_write(code, &new, sizeof(new));
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 0);
+	jump_label_transform(entry, type);
 	text_poke_sync();
 }
 
 bool arch_jump_label_transform_queue(struct jump_entry *entry,
 				     enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 0);
+	jump_label_transform(entry, type);
 	return true;
 }
 
@@ -95,6 +84,4 @@ void arch_jump_label_transform_apply(void)
 void __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
 						       enum jump_label_type type)
 {
-	jump_label_transform(entry, type, 1);
-	text_poke_sync();
 }

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-06-28  7:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 15:41 [PATCH v2 0/3] jump_label: get rid of NOP patching where possible Ard Biesheuvel
2022-06-15 15:41 ` [PATCH v2 1/3] jump_label: s390: avoid pointless initial NOP patching Ard Biesheuvel
2022-06-26  7:57   ` Alexander Gordeev
2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
2022-06-15 15:41 ` [PATCH v2 2/3] jump_label: mips: move module NOP patching into arch code Ard Biesheuvel
2022-06-26  8:03   ` Alexander Gordeev
2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel
2022-06-15 15:41 ` [PATCH v2 3/3] jump_label: make initial NOP patching the special case Ard Biesheuvel
2022-06-16 11:25   ` Ard Biesheuvel
2022-06-17 13:40     ` Peter Zijlstra
2022-06-26  8:04   ` Alexander Gordeev
2022-06-28  7:09   ` [tip: locking/core] " tip-bot2 for Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).