Messages in this thread | | | From | Nick Desaulniers <> | Date | Wed, 13 Oct 2021 13:11:45 -0700 | Subject | Re: [PATCH 5/9] x86/alternative: Handle Jcc __x86_indirect_thunk_\reg |
| |
On Wed, Oct 13, 2021 at 5:41 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Handle the rare cases where the compiler (clang) does an indirect > conditional tail-call using: > > Jcc __x86_indirect_thunk_\reg
`Jcc.d32 __x86_indirect_thunk_\reg` might be clearer; otherwise putting that in an assembler and assembling/disassembling produces the 2B instructions, which makes the below patch confusing. Ah, it is stated in the comment added below.
> > For the !RETPOLINE case this can be rewritten to fit the original (6 > byte) instruction like: > > Jncc.d8 1f > JMP *%\reg > NOP > 1: > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/x86/kernel/alternative.c | 38 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 34 insertions(+), 4 deletions(-) > > --- a/arch/x86/kernel/alternative.c > +++ b/arch/x86/kernel/alternative.c > @@ -378,7 +378,8 @@ static int emit_indirect(int op, int reg > static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes) > { > void (*target)(void); > - int reg, i = 0; > + int reg, ret, i = 0; > + u8 op, cc; > > if (cpu_feature_enabled(X86_FEATURE_RETPOLINE)) > return -1; > @@ -390,9 +391,34 @@ static int patch_retpoline(void *addr, s > if (WARN_ON_ONCE(reg & ~0xf)) > return -1; > > - i = emit_indirect(insn->opcode.bytes[0], reg, bytes); > - if (i < 0) > - return i; > + op = insn->opcode.bytes[0]; > + > + /* > + * Convert: > + * > + * Jcc.d32 __x86_indirect_thunk_\reg > + * > + * into: > + * > + * Jncc.d8 1f > + * jmp *%\reg > + * nop > + * 1: > + */ > + if (op == 0x0f && (insn->opcode.bytes[1] & 0xf0) == 0x80) { > + cc = insn->opcode.bytes[1] & 0xf; > + cc ^= 1; /* invert condition */ > + > + bytes[i++] = 0x70 + cc; /* Jcc.d8 */ > + bytes[i++] = insn->length - 2;
Isn't `insn->length - 2` always 4 (in this case)? We could avoid computing that at runtime I suspect if we just hardcoded it.
Either way, I've looked at the disassembly enough that this LGTM. Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> + > + op = JMP32_INSN_OPCODE; > + } > + > + ret = emit_indirect(op, reg, bytes + i); > + if (ret < 0) > + return ret; > + i += ret; > > for (; i < insn->length;) > bytes[i++] = BYTES_NOP1; > @@ -423,6 +449,10 @@ void __init_or_module noinline apply_ret > case JMP32_INSN_OPCODE: > break; > > + case 0x0f: /* escape */ > + if (op2 >= 0x80 && op2 <= 0x8f) > + break; > + fallthrough; > default: > WARN_ON_ONCE(1); > continue; > >
-- Thanks, ~Nick Desaulniers
| |