Messages in this thread | | | Date | Mon, 25 Oct 2021 17:03:00 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/4] arm64: implement support for static call trampolines |
| |
On Mon, Oct 25, 2021 at 04:55:17PM +0200, Ard Biesheuvel wrote: > On Mon, 25 Oct 2021 at 16:47, Peter Zijlstra <peterz@infradead.org> wrote:
> > Perhaps a little something like so.. Shaves 2 instructions off each > > trampoline. > > > > --- a/arch/arm64/include/asm/static_call.h > > +++ b/arch/arm64/include/asm/static_call.h > > @@ -11,9 +11,7 @@ > > " hint 34 /* BTI C */ \n" \ > > insn " \n" \ > > " ldr x16, 0b \n" \ > > - " cbz x16, 1f \n" \ > > " br x16 \n" \ > > - "1: ret \n" \ > > " .popsection \n") > > > > #define ARCH_DEFINE_STATIC_CALL_TRAMP(name, func) \ > > --- a/arch/arm64/kernel/patching.c > > +++ b/arch/arm64/kernel/patching.c > > @@ -90,6 +90,11 @@ int __kprobes aarch64_insn_write(void *a > > return __aarch64_insn_write(addr, &i, AARCH64_INSN_SIZE); > > } > > > > +asm("__static_call_ret: \n" > > + " ret \n") > > + > > This breaks BTI as it lacks the landing pad, and it will be called indirectly.
Argh!
> > +extern void __static_call_ret(void); > > + > > Better to have an ordinary C function here (with consistent linkage), > but we need to take the address in a way that works with Clang CFI.
There is that.
> As the two additional instructions are on an ice cold path anyway, I'm > not sure this is an obvious improvement tbh.
For me it's both simpler -- by virtue of being more consistent, and smaller. So double win :-)
That is; you're already relying on the literal being unconditionally updated for the normal B foo -> NOP path, and having the RET -> NOP path be handled differently is just confusing.
At least, that's how I'm seeing it today...
| |