* [PATCH 1/2] MIPS: jump_label.c: Correct the span of the J instruction @ 2014-11-17 16:09 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-17 16:09 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips Correct the check for the span of the 256MB segment addressable by the J instruction according to this instruction's semantics. The calculation of the jump target is applied to the address of the delay-slot instruction that immediately follows. Adjust the check accordingly by adding 4 to `e->code' that holds the address of the J instruction itself. Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> --- Hi, I wonder why people still make this mistake, now that the architecture has been around for nearly 30 years now... Please apply. Maciej linux-mips-jump-label-range.diff Index: linux-3.17-stable-malta/arch/mips/kernel/jump_label.c =================================================================== --- linux-3.17-stable-malta.orig/arch/mips/kernel/jump_label.c 2014-11-17 02:12:17.000000000 +0000 +++ linux-3.17-stable-malta/arch/mips/kernel/jump_label.c 2014-11-17 02:22:04.741976773 +0000 @@ -27,8 +27,8 @@ void arch_jump_label_transform(struct ju union mips_instruction *insn_p = (union mips_instruction *)(unsigned long)e->code; - /* Jump only works within a 256MB aligned region. */ - BUG_ON((e->target & ~J_RANGE_MASK) != (e->code & ~J_RANGE_MASK)); + /* Jump only works within a 256MB aligned region of its delay slot. */ + BUG_ON((e->target & ~J_RANGE_MASK) != ((e->code + 4) & ~J_RANGE_MASK)); /* Target must have 4 byte alignment. */ BUG_ON((e->target & 3) != 0); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] MIPS: jump_label.c: Correct the span of the J instruction @ 2014-11-17 16:09 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-17 16:09 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips Correct the check for the span of the 256MB segment addressable by the J instruction according to this instruction's semantics. The calculation of the jump target is applied to the address of the delay-slot instruction that immediately follows. Adjust the check accordingly by adding 4 to `e->code' that holds the address of the J instruction itself. Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> --- Hi, I wonder why people still make this mistake, now that the architecture has been around for nearly 30 years now... Please apply. Maciej linux-mips-jump-label-range.diff Index: linux-3.17-stable-malta/arch/mips/kernel/jump_label.c =================================================================== --- linux-3.17-stable-malta.orig/arch/mips/kernel/jump_label.c 2014-11-17 02:12:17.000000000 +0000 +++ linux-3.17-stable-malta/arch/mips/kernel/jump_label.c 2014-11-17 02:22:04.741976773 +0000 @@ -27,8 +27,8 @@ void arch_jump_label_transform(struct ju union mips_instruction *insn_p = (union mips_instruction *)(unsigned long)e->code; - /* Jump only works within a 256MB aligned region. */ - BUG_ON((e->target & ~J_RANGE_MASK) != (e->code & ~J_RANGE_MASK)); + /* Jump only works within a 256MB aligned region of its delay slot. */ + BUG_ON((e->target & ~J_RANGE_MASK) != ((e->code + 4) & ~J_RANGE_MASK)); /* Target must have 4 byte alignment. */ BUG_ON((e->target & 3) != 0); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding @ 2014-11-17 16:10 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-17 16:10 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips Implement the microMIPS encoding of the J instruction for the purpose of the static keys feature, fixing a crash early on in bootstrap as the kernel is unhappy seeing the ISA bit set in jump table entries. Make sure the ISA bit correctly reflects the instruction encoding chosen for the kernel, 0 for the standard MIPS and 1 for the microMIPS encoding. Also make sure the instruction to patch is a 32-bit NOP in the microMIPS mode as by default the 16-bit short encoding is assumed Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> --- Hi, Rather obvious, mostly a bit different bit shuffling. I think all code that reads or writes microMIPS instructions should be audited and the operations abstracted so that every place does not have to repeat the same steps manually, but that is something for another occasion. Meanwhile, please apply. Maciej linux-umips-jump-label.diff Index: linux-3.17-stable-malta-el/arch/mips/include/asm/jump_label.h =================================================================== --- linux-3.17-stable-malta-el.orig/arch/mips/include/asm/jump_label.h 2014-11-17 02:12:17.000000000 +0000 +++ linux-3.17-stable-malta-el/arch/mips/include/asm/jump_label.h 2014-11-17 07:06:18.491909609 +0000 @@ -20,9 +20,15 @@ #define WORD_INSN ".word" #endif +#ifdef CONFIG_CPU_MICROMIPS +#define NOP_INSN "nop32" +#else +#define NOP_INSN "nop" +#endif + static __always_inline bool arch_static_branch(struct static_key *key) { - asm_volatile_goto("1:\tnop\n\t" + asm_volatile_goto("1:\t" NOP_INSN "\n\t" "nop\n\t" ".pushsection __jump_table, \"aw\"\n\t" WORD_INSN " 1b, %l[l_yes], %0\n\t" Index: linux-3.17-stable-malta-el/arch/mips/kernel/jump_label.c =================================================================== --- linux-3.17-stable-malta-el.orig/arch/mips/kernel/jump_label.c 2014-11-17 07:06:18.000000000 +0000 +++ linux-3.17-stable-malta-el/arch/mips/kernel/jump_label.c 2014-11-17 08:28:23.781948719 +0000 @@ -18,31 +18,53 @@ #ifdef HAVE_JUMP_LABEL -#define J_RANGE_MASK ((1ul << 28) - 1) +/* + * Define parameters for the standard MIPS and the microMIPS jump + * instruction encoding respectively: + * + * - the ISA bit of the target, either 0 or 1 respectively, + * + * - the amount the jump target address is shifted right to fit in the + * immediate field of the machine instruction, either 2 or 1, + * + * - the mask determining the size of the jump region relative to the + * delay-slot instruction, either 256MB or 128MB, + * + * - the jump target alignment, either 4 or 2 bytes. + */ +#define J_ISA_BIT IS_ENABLED(CONFIG_CPU_MICROMIPS) +#define J_RANGE_SHIFT (2 - J_ISA_BIT) +#define J_RANGE_MASK ((1ul << (26 + J_RANGE_SHIFT)) - 1) +#define J_ALIGN_MASK ((1ul << J_RANGE_SHIFT) - 1) void arch_jump_label_transform(struct jump_entry *e, enum jump_label_type type) { + union mips_instruction *insn_p; union mips_instruction insn; - union mips_instruction *insn_p = - (union mips_instruction *)(unsigned long)e->code; - /* Jump only works within a 256MB aligned region of its delay slot. */ + insn_p = (union mips_instruction *)msk_isa16_mode(e->code); + + /* Jump only works within an aligned region its delay slot is in. */ BUG_ON((e->target & ~J_RANGE_MASK) != ((e->code + 4) & ~J_RANGE_MASK)); - /* Target must have 4 byte alignment. */ - BUG_ON((e->target & 3) != 0); + /* Target must have the right alignment and ISA must be preserved. */ + BUG_ON((e->target & J_ALIGN_MASK) != J_ISA_BIT); if (type == JUMP_LABEL_ENABLE) { - insn.j_format.opcode = j_op; - insn.j_format.target = (e->target & J_RANGE_MASK) >> 2; + insn.j_format.opcode = J_ISA_BIT ? mm_j32_op : j_op; + insn.j_format.target = e->target >> J_RANGE_SHIFT; } else { insn.word = 0; /* nop */ } get_online_cpus(); mutex_lock(&text_mutex); - *insn_p = insn; + if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) { + insn_p->halfword[0] = insn.word >> 16; + insn_p->halfword[1] = insn.word; + } else + *insn_p = insn; flush_icache_range((unsigned long)insn_p, (unsigned long)insn_p + sizeof(*insn_p)); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding @ 2014-11-17 16:10 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-17 16:10 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips Implement the microMIPS encoding of the J instruction for the purpose of the static keys feature, fixing a crash early on in bootstrap as the kernel is unhappy seeing the ISA bit set in jump table entries. Make sure the ISA bit correctly reflects the instruction encoding chosen for the kernel, 0 for the standard MIPS and 1 for the microMIPS encoding. Also make sure the instruction to patch is a 32-bit NOP in the microMIPS mode as by default the 16-bit short encoding is assumed Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> --- Hi, Rather obvious, mostly a bit different bit shuffling. I think all code that reads or writes microMIPS instructions should be audited and the operations abstracted so that every place does not have to repeat the same steps manually, but that is something for another occasion. Meanwhile, please apply. Maciej linux-umips-jump-label.diff Index: linux-3.17-stable-malta-el/arch/mips/include/asm/jump_label.h =================================================================== --- linux-3.17-stable-malta-el.orig/arch/mips/include/asm/jump_label.h 2014-11-17 02:12:17.000000000 +0000 +++ linux-3.17-stable-malta-el/arch/mips/include/asm/jump_label.h 2014-11-17 07:06:18.491909609 +0000 @@ -20,9 +20,15 @@ #define WORD_INSN ".word" #endif +#ifdef CONFIG_CPU_MICROMIPS +#define NOP_INSN "nop32" +#else +#define NOP_INSN "nop" +#endif + static __always_inline bool arch_static_branch(struct static_key *key) { - asm_volatile_goto("1:\tnop\n\t" + asm_volatile_goto("1:\t" NOP_INSN "\n\t" "nop\n\t" ".pushsection __jump_table, \"aw\"\n\t" WORD_INSN " 1b, %l[l_yes], %0\n\t" Index: linux-3.17-stable-malta-el/arch/mips/kernel/jump_label.c =================================================================== --- linux-3.17-stable-malta-el.orig/arch/mips/kernel/jump_label.c 2014-11-17 07:06:18.000000000 +0000 +++ linux-3.17-stable-malta-el/arch/mips/kernel/jump_label.c 2014-11-17 08:28:23.781948719 +0000 @@ -18,31 +18,53 @@ #ifdef HAVE_JUMP_LABEL -#define J_RANGE_MASK ((1ul << 28) - 1) +/* + * Define parameters for the standard MIPS and the microMIPS jump + * instruction encoding respectively: + * + * - the ISA bit of the target, either 0 or 1 respectively, + * + * - the amount the jump target address is shifted right to fit in the + * immediate field of the machine instruction, either 2 or 1, + * + * - the mask determining the size of the jump region relative to the + * delay-slot instruction, either 256MB or 128MB, + * + * - the jump target alignment, either 4 or 2 bytes. + */ +#define J_ISA_BIT IS_ENABLED(CONFIG_CPU_MICROMIPS) +#define J_RANGE_SHIFT (2 - J_ISA_BIT) +#define J_RANGE_MASK ((1ul << (26 + J_RANGE_SHIFT)) - 1) +#define J_ALIGN_MASK ((1ul << J_RANGE_SHIFT) - 1) void arch_jump_label_transform(struct jump_entry *e, enum jump_label_type type) { + union mips_instruction *insn_p; union mips_instruction insn; - union mips_instruction *insn_p = - (union mips_instruction *)(unsigned long)e->code; - /* Jump only works within a 256MB aligned region of its delay slot. */ + insn_p = (union mips_instruction *)msk_isa16_mode(e->code); + + /* Jump only works within an aligned region its delay slot is in. */ BUG_ON((e->target & ~J_RANGE_MASK) != ((e->code + 4) & ~J_RANGE_MASK)); - /* Target must have 4 byte alignment. */ - BUG_ON((e->target & 3) != 0); + /* Target must have the right alignment and ISA must be preserved. */ + BUG_ON((e->target & J_ALIGN_MASK) != J_ISA_BIT); if (type == JUMP_LABEL_ENABLE) { - insn.j_format.opcode = j_op; - insn.j_format.target = (e->target & J_RANGE_MASK) >> 2; + insn.j_format.opcode = J_ISA_BIT ? mm_j32_op : j_op; + insn.j_format.target = e->target >> J_RANGE_SHIFT; } else { insn.word = 0; /* nop */ } get_online_cpus(); mutex_lock(&text_mutex); - *insn_p = insn; + if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) { + insn_p->halfword[0] = insn.word >> 16; + insn_p->halfword[1] = insn.word; + } else + *insn_p = insn; flush_icache_range((unsigned long)insn_p, (unsigned long)insn_p + sizeof(*insn_p)); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding 2014-11-17 16:10 ` Maciej W. Rozycki (?) @ 2014-11-19 9:27 ` Ralf Baechle 2014-11-19 11:37 ` Maciej W. Rozycki -1 siblings, 1 reply; 7+ messages in thread From: Ralf Baechle @ 2014-11-19 9:27 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: linux-mips On Mon, Nov 17, 2014 at 04:10:32PM +0000, Maciej W. Rozycki wrote: > + if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) { > + insn_p->halfword[0] = insn.word >> 16; > + insn_p->halfword[1] = insn.word; > + } else > + *insn_p = insn; I think the IS_ENABLED() should be replaced with cpu_has_mmips? Ralf ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding @ 2014-11-19 11:37 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-19 11:37 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips On Wed, 19 Nov 2014, Ralf Baechle wrote: > > + if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) { > > + insn_p->halfword[0] = insn.word >> 16; > > + insn_p->halfword[1] = insn.word; > > + } else > > + *insn_p = insn; > > I think the IS_ENABLED() should be replaced with cpu_has_mmips? Nope, this leg must only execute if the kernel itself has been built as microMIPS code -- it's patching itself here. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding @ 2014-11-19 11:37 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2014-11-19 11:37 UTC (permalink / raw) To: Ralf Baechle; +Cc: linux-mips On Wed, 19 Nov 2014, Ralf Baechle wrote: > > + if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) { > > + insn_p->halfword[0] = insn.word >> 16; > > + insn_p->halfword[1] = insn.word; > > + } else > > + *insn_p = insn; > > I think the IS_ENABLED() should be replaced with cpu_has_mmips? Nope, this leg must only execute if the kernel itself has been built as microMIPS code -- it's patching itself here. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-19 11:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-17 16:09 [PATCH 1/2] MIPS: jump_label.c: Correct the span of the J instruction Maciej W. Rozycki 2014-11-17 16:09 ` Maciej W. Rozycki 2014-11-17 16:10 ` [PATCH 2/2] MIPS: jump_label.c: Handle the microMIPS J instruction encoding Maciej W. Rozycki 2014-11-17 16:10 ` Maciej W. Rozycki 2014-11-19 9:27 ` Ralf Baechle 2014-11-19 11:37 ` Maciej W. Rozycki 2014-11-19 11:37 ` Maciej W. Rozycki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.