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