Björn Töpel <bj...@kernel.org> writes: > Andy Chiu <andy.c...@sifive.com> writes: > >> We use an AUIPC+JALR pair to jump into a ftrace trampoline. Since >> instruction fetch can break down to 4 byte at a time, it is impossible >> to update two instructions without a race. In order to mitigate it, we >> initialize the patchable entry to AUIPC + NOP4. Then, the run-time code >> patching can change NOP4 to JALR to eable/disable ftrcae from a > enable ftrace > >> function. This limits the reach of each ftrace entry to +-2KB displacing >> from ftrace_caller. >> >> Starting from the trampoline, we add a level of indirection for it to >> reach ftrace caller target. Now, it loads the target address from a >> memory location, then perform the jump. This enable the kernel to update >> the target atomically. > > The +-2K limit is for direct calls, right? > > ...and this I would say breaks DIRECT_CALLS (which should be implemented > using call_ops later)?
Thinking a bit more, and re-reading the series. This series is good work, and it's a big improvement for DYNAMIC_FTRACE, but +int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) +{ + unsigned long distance, orig_addr; + + orig_addr = (unsigned long)&ftrace_caller; + distance = addr > orig_addr ? addr - orig_addr : orig_addr - addr; + if (distance > JALR_RANGE) + return -EINVAL; + + return __ftrace_modify_call(rec->ip, addr, false); +} + breaks WITH_DIRECT_CALLS. The direct trampoline will *never* be within the JALR_RANGE. Unless we're happy with a break (I'm not) -- I really think Puranjay's CALL_OPS patch needs to be baked in in the series! Björn