On Mon, 2013-08-05 at 17:28 -0400, Mathieu Desnoyers wrote: > Another thing that bothers me with Steven's approach is that decoding > jumps generated by the compiler seems fragile IMHO.
The encodings wont change. If they do, then old kernels will not run on new hardware. Now if it adds a third option to jmp, then we hit the "die" path and know right away that it wont work anymore. Then we fix it properly. > > x86 decoding proposed by https://lkml.org/lkml/2012/3/8/464 : > > +static int make_nop_x86(void *map, size_t const offset) > +{ > + unsigned char *op; > + unsigned char *nop; > + int size; > + > + /* Determine which type of jmp this is 2 byte or 5. */ > + op = map + offset; > + switch (*op) { > + case 0xeb: /* 2 byte */ > + size = 2; > + nop = ideal_nop2_x86; > + break; > + case 0xe9: /* 5 byte */ > + size = 5; > + nop = ideal_nop; > + break; > + default: > + die(NULL, "Bad jump label section (bad op %x)\n", *op); > + __builtin_unreachable(); > + } > > My though is that the code above does not cover all jump encodings that > can be generated by past, current and future x86 assemblers. > > Another way around this issue might be to keep the instruction size > within a non-allocated section: > > static __always_inline bool arch_static_branch(struct static_key *key) > { > asm goto("1:" > "jmp %l[l_yes]\n\t" > "2:" > > ".pushsection __jump_table, \"aw\" \n\t" > _ASM_ALIGN "\n\t" > _ASM_PTR "1b, %l[l_yes], %c0 \n\t" > ".popsection \n\t" > > ".pushsection __jump_table_ilen \n\t" > _ASM_PTR "1b \n\t" /* Address of the jmp */ > ".byte 2b - 1b \n\t" /* Size of the jmp instruction */ > ".popsection \n\t" > > : : "i" (key) : : l_yes); > return false; > l_yes: > return true; > } > > And use (2b - 1b) to know what size of no-op should be used rather than > to rely on instruction decoding. > > Thoughts ? > Then we need to add yet another table of information to the kernel that needs to hang around. This goes with another kernel-discuss request talking about kernel data bloat. -- Steve