On Mon, 8 Oct 2018 23:55:34 -0400 Steven Rostedt <rost...@goodmis.org> wrote:
> On Tue, 9 Oct 2018 12:44:01 +0900 > Masami Hiramatsu <mhira...@kernel.org> wrote: > > > On Fri, 05 Oct 2018 21:51:11 -0400 > > Steven Rostedt <rost...@goodmis.org> wrote: > > > > > +typedef long dynfunc_t; > > > + > > > +struct dynfunc_struct; > > > + > > > +#define arch_dynfunc_trampoline(name, def) \ > > > + asm volatile ( \ > > > + ".globl dynfunc_" #name "; \n\t" \ > > > + "dynfunc_" #name ": \n\t" \ > > > + "jmp " #def " \n\t" \ > > > + ".balign 8 \n \t" \ > > > + : : : "memory" ) > > > + > > > > I have just a question, what is this different from livepatch? :) > > I actually thought about this a bit, but decided against it. > > I didn't want to hook another infrastructure into the fentry nop. It's > already complex enough with kprobes, live patching and ftrace. > > The ideal solution is what Peter suggested, and that's to patch the > call sites, and I think that is attainable with objtool modifications. OK, the ideal solution sounds good to me. > > > > > I think we can replace the first 5 bytes of the default function > > to jmp instruction (to alternative function) instead of making > > this trampoline. > > > > IOW, as far as I can see, this is changing > > > > ---- > > call %reg (or retpoline_reg) > > ---- > > > > to > > > > ---- > > call dynfunc_A > > > > dynfunc_A: > > jmp func_A or altered_func_A > > ---- > > > > If so, why don't we put the jmp on default func_A directly? > > ---- > > call func_A > > > > func_A: > > "jmp altered_func" or "original sequence" > > ---- > > (this is idealy same as jprobes did) > > > > Of course we have to arbitrate it with ftrace (fentry) but it may > > not so hard (simplest way is just adding "notrace" on the default > > function) > > Then we lose the 5 byte nop. Yeah, but we can remove the trampoline code. > > BTW, I think "dynamic_function" may not correct name, it may be > > "alternative_function" or something like that, because this > > function must be replaced system-wide and this means we can > > not use this for generic function pointer usage which depends > > on thread context (like file_operations). But good for something > > pluggable code (LSM?). > > I don't like the name alternative, as that's usually a one shot deal > (SMP vs UP). > > It is dynamic, as it's a function that changes dynamically. Yes its > global, but that's not mutually exclusive to dynamic. OK, so we may add a note that this is "global" patching :) > The use case I want this for is for tracing. But it can be useful for > KVM and power management governors. Basically anything that has a > global function pointer (hmm, even the idle call can use this). Indeed. Thanks, > > -- Steve -- Masami Hiramatsu <mhira...@kernel.org>
