On 2017/06/15 09:19PM, Michael Ellerman wrote: > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> writes: > > > diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > > b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > > index fa0921410fa4..e6837e85ec28 100644 > > --- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > > +++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > > @@ -99,13 +99,37 @@ ftrace_call: > > bl ftrace_stub > > nop > > > > - /* Load ctr with the possibly modified NIP */ > > - ld r3, _NIP(r1) > > - mtctr r3 > > + /* Load the possibly modified NIP */ > > + ld r15, _NIP(r1) > > + > > #ifdef CONFIG_LIVEPATCH > > - cmpd r14,r3 /* has NIP been altered? */ > > + cmpd r14, r15 /* has NIP been altered? */ > > +#endif > > + > > +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_KPROBES_ON_FTRACE) > > + beq 1f > > + > > + /* Check if there is an active kprobe on us */ > > + subi r3, r14, 4 > > + bl is_current_kprobe_addr > > + nop > > + > > + /* > > + * If r3 == 1, then this is a kprobe/jprobe. > > + * else, this is livepatched function. > > + * > > + * The subsequent conditional branch for livepatch_handler > > + * will use the result of this compare. For kprobe/jprobe, > > + * we just need to branch to the new NIP, so nothing special > > + * is needed. > > + */ > > + cmpdi r3, 1 > > +1: > > #endif > > I added some more comments. Hopefully I got them right :D > > #if defined(CONFIG_LIVEPATCH) && defined(CONFIG_KPROBES_ON_FTRACE) > + /* NIP has not been altered, skip over further checks */ > beq 1f > > /* Check if there is an active kprobe on us */ > @@ -118,10 +119,11 @@ ftrace_call: > * If r3 == 1, then this is a kprobe/jprobe. > * else, this is livepatched function. > * > - * The subsequent conditional branch for livepatch_handler > - * will use the result of this compare. For kprobe/jprobe, > - * we just need to branch to the new NIP, so nothing special > - * is needed. > + * The conditional branch for livepatch_handler below will use the > + * result of this comparison. For kprobe/jprobe, we just need to > branch to > + * the new NIP, not call livepatch_handler. The branch below is bne, > so we > + * want CR0[EQ] to be true if this is a kprobe/jprobe. Which means we > want > + * CR0[EQ] = (r3 == 1). > */ > cmpdi r3, 1 > 1: > @@ -147,7 +149,10 @@ ftrace_call: > addi r1, r1, SWITCH_FRAME_SIZE > > #ifdef CONFIG_LIVEPATCH > - /* Based on the cmpd above, if the NIP was altered handle livepatch > */ > + /* > + * Based on the cmpd or cmpdi above, if the NIP was altered and we're > + * not on a kprobe/jprobe, then handle livepatch. > + */ > bne- livepatch_handler > #endif
Oh yes, this makes the code logic a whole lot more clearer and explicit. Thanks for expanding on it! - Naveen
