| Message ID | 20220615154142.1574619-4-ardb@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | jump_label: get rid of NOP patching where possible | expand |
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.
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!
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 >
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) {