| Message ID | 1347875671-15838-2-git-send-email-tiejun.chen@windriver.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
> /* N.B. the only way to get here is from the beq following ret_from_except. */ > resume_kernel: > - /* check current_thread_info->preempt_count */ > + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ > CURRENT_THREAD_INFO(r9, r1) > + lwz r8,TI_FLAGS(r9) > + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h > + beq+ 1f ... > +1: Does this add a statically mispredicted branch to every return to userspace ? Or is there an earlier check for 'unlikely' conditions. David
On 09/17/2012 06:02 PM, David Laight wrote: >> /* N.B. the only way to get here is from the beq following ret_from_except. */ >> resume_kernel: >> - /* check current_thread_info->preempt_count */ >> + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ >> CURRENT_THREAD_INFO(r9, r1) >> + lwz r8,TI_FLAGS(r9) >> + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h >> + beq+ 1f > ... >> +1: > > Does this add a statically mispredicted branch to every > return to userspace ? Return usersapce? No, this is just follow 'resume_kernel'. Note I add this 'unlikely' here since I assume often Kprobe is always disabled by default and especially its also rare to kprobe 'stwu' in many kprobe cases. Tiejun
On Mon, 2012-09-17 at 17:54 +0800, Tiejun Chen wrote: > -#ifdef CONFIG_PREEMPT > b restore > > /* N.B. the only way to get here is from the beq following ret_from_except. */ > resume_kernel: > - /* check current_thread_info->preempt_count */ > + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ > CURRENT_THREAD_INFO(r9, r1) > + lwz r8,TI_FLAGS(r9) > + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h > + beq+ 1f > + > + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ > + > + lwz r3,GPR1(r1) > + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ > + mr r4,r1 /* src: current exception frame */ > + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ > + li r6,0 /* start offset: 0 */ > + mr r1,r3 /* Reroute the trampoline frame to r1 */ > + > + /* Copy from the original to the trampoline. */ > + li r6,0 You just did that li r6,0 2 lines above :-) I'll fix it up manually while applying. > + srwi r5,r5,2 > + mtctr r5 > +2: lwzx r0,r6,r4 > + stwx r0,r6,r3 > + addi r6,r6,4 > + bdnz 2b > + > + /* Do real store operation to complete stwu */ > + lwz r5,GPR1(r1) > + stw r8,0(r5) > + > + /* Clear _TIF_EMULATE_STACK_STORE flag */ > + lis r11,_TIF_EMULATE_STACK_STORE@h > + addi r5,r9,TI_FLAGS > +0: lwarx r8,0,r5 > + andc r8,r8,r11 > +#ifdef CONFIG_IBM405_ERR77 > + dcbt 0,r5 > +#endif > + stwcx. r8,0,r5 > + bne- 0b > +1: > + > +#ifdef CONFIG_PREEMPT > + /* check current_thread_info->preempt_count */ > lwz r0,TI_PREEMPT(r9) > cmpwi 0,r0,0 /* if non-zero, just restore regs and return */ > bne restore > - lwz r0,TI_FLAGS(r9) > - andi. r0,r0,_TIF_NEED_RESCHED > + andi. r8,r8,_TIF_NEED_RESCHED > beq+ restore > + lwz r3,_MSR(r1) > andi. r0,r3,MSR_EE /* interrupts off? */ > beq restore /* don't schedule if so */ > #ifdef CONFIG_TRACE_IRQFLAGS > @@ -864,8 +903,6 @@ resume_kernel: > */ > bl trace_hardirqs_on > #endif > -#else > -resume_kernel: > #endif /* CONFIG_PREEMPT */ > > /* interrupts are hard-disabled at this point */ > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S > index b40e0b4..bdd2dc1 100644 > --- a/arch/powerpc/kernel/entry_64.S > +++ b/arch/powerpc/kernel/entry_64.S > @@ -593,6 +593,43 @@ _GLOBAL(ret_from_except_lite) > b .ret_from_except > > resume_kernel: > + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ > + CURRENT_THREAD_INFO(r9, r1) > + ld r8,TI_FLAGS(r9) > + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h > + beq+ 1f > + > + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ > + > + lwz r3,GPR1(r1) > + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ > + mr r4,r1 /* src: current exception frame */ > + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ > + li r6,0 /* start offset: 0 */ > + mr r1,r3 /* Reroute the trampoline frame to r1 */ > + > + /* Copy from the original to the trampoline. */ > + li r6,0 > + srwi r5,r5,3 > + mtctr r5 > +2: ldx r0,r6,r4 > + stdx r0,r6,r3 > + addi r6,r6,8 > + bdnz 2b > + > + /* Do real store operation to complete stwu */ > + lwz r5,GPR1(r1) > + std r8,0(r5) > + > + /* Clear _TIF_EMULATE_STACK_STORE flag */ > + lis r11,_TIF_EMULATE_STACK_STORE@h > + addi r5,r9,TI_FLAGS > + ldarx r4,0,r5 > + andc r4,r4,r11 > + stdcx. r4,0,r5 > + bne- 0b > +1: > + > #ifdef CONFIG_PREEMPT > /* Check if we need to preempt */ > andi. r0,r4,_TIF_NEED_RESCHED
On Tue, 2012-09-18 at 15:05 +1000, Benjamin Herrenschmidt wrote: > On Mon, 2012-09-17 at 17:54 +0800, Tiejun Chen wrote: > > -#ifdef CONFIG_PREEMPT > > b restore > > > > /* N.B. the only way to get here is from the beq following ret_from_except. */ > > resume_kernel: > > - /* check current_thread_info->preempt_count */ > > + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ > > CURRENT_THREAD_INFO(r9, r1) > > + lwz r8,TI_FLAGS(r9) > > + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h > > + beq+ 1f > > + > > + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ > > + > > + lwz r3,GPR1(r1) > > + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ > > + mr r4,r1 /* src: current exception frame */ > > + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ > > + li r6,0 /* start offset: 0 */ > > + mr r1,r3 /* Reroute the trampoline frame to r1 */ > > + > > + /* Copy from the original to the trampoline. */ > > + li r6,0 > > You just did that li r6,0 2 lines above :-) I'll fix it up manually > while applying. In fact the srwi can be dropped completely, we can just load r5 with the divided value. Committed, will push later today, please test. Cheers, Ben.
On 09/18/2012 01:09 PM, Benjamin Herrenschmidt wrote: > On Tue, 2012-09-18 at 15:05 +1000, Benjamin Herrenschmidt wrote: >> On Mon, 2012-09-17 at 17:54 +0800, Tiejun Chen wrote: >>> -#ifdef CONFIG_PREEMPT >>> b restore >>> >>> /* N.B. the only way to get here is from the beq following ret_from_except. */ >>> resume_kernel: >>> - /* check current_thread_info->preempt_count */ >>> + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ >>> CURRENT_THREAD_INFO(r9, r1) >>> + lwz r8,TI_FLAGS(r9) >>> + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h >>> + beq+ 1f >>> + >>> + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ >>> + >>> + lwz r3,GPR1(r1) >>> + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ >>> + mr r4,r1 /* src: current exception frame */ >>> + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ >>> + li r6,0 /* start offset: 0 */ >>> + mr r1,r3 /* Reroute the trampoline frame to r1 */ >>> + >>> + /* Copy from the original to the trampoline. */ >>> + li r6,0 >> >> You just did that li r6,0 2 lines above :-) I'll fix it up manually >> while applying. > > In fact the srwi can be dropped completely, we can just load r5 with the > divided value. Committed, will push later today, please test. I retest to kprobe do_fork() and show_interrupts() with/without enabling CONFIG_PREEMPT, separately, looks still work. For 32-bit: ------------ + /* Copy from the original to the trampoline. */ + lwz r3,GPR1(r1) + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ + mr r4,r1 /* src: current exception frame */ + li r5,INT_FRAME_SIZE/4 /* size: INT_FRAME_SIZE */ + li r6,0 /* start offset: 0 */ + mr r1,r3 /* Reroute the trampoline frame to r1 */ + mtctr r5 +2: lwzx r0,r6,r4 + stwx r0,r6,r3 + addi r6,r6,4 + bdnz 2b And for 64-bit: --------------- + /* Copy from the original to the trampoline. */ + lwz r3,GPR1(r1) + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ + mr r4,r1 /* src: current exception frame */ + li r5,INT_FRAME_SIZE/8 /* size: INT_FRAME_SIZE */ + li r6,0 /* start offset: 0 */ + mr r1,r3 /* Reroute the trampoline frame to r1 */ + mtctr r5 +2: ldx r0,r6,r4 + stdx r0,r6,r3 + addi r6,r6,8 + bdnz 2b Thanks Tiejun
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index ead5016..d27fe36 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -831,19 +831,58 @@ restore_user: bnel- load_dbcr0 #endif -#ifdef CONFIG_PREEMPT b restore /* N.B. the only way to get here is from the beq following ret_from_except. */ resume_kernel: - /* check current_thread_info->preempt_count */ + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ CURRENT_THREAD_INFO(r9, r1) + lwz r8,TI_FLAGS(r9) + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h + beq+ 1f + + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ + + lwz r3,GPR1(r1) + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ + mr r4,r1 /* src: current exception frame */ + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ + li r6,0 /* start offset: 0 */ + mr r1,r3 /* Reroute the trampoline frame to r1 */ + + /* Copy from the original to the trampoline. */ + li r6,0 + srwi r5,r5,2 + mtctr r5 +2: lwzx r0,r6,r4 + stwx r0,r6,r3 + addi r6,r6,4 + bdnz 2b + + /* Do real store operation to complete stwu */ + lwz r5,GPR1(r1) + stw r8,0(r5) + + /* Clear _TIF_EMULATE_STACK_STORE flag */ + lis r11,_TIF_EMULATE_STACK_STORE@h + addi r5,r9,TI_FLAGS +0: lwarx r8,0,r5 + andc r8,r8,r11 +#ifdef CONFIG_IBM405_ERR77 + dcbt 0,r5 +#endif + stwcx. r8,0,r5 + bne- 0b +1: + +#ifdef CONFIG_PREEMPT + /* check current_thread_info->preempt_count */ lwz r0,TI_PREEMPT(r9) cmpwi 0,r0,0 /* if non-zero, just restore regs and return */ bne restore - lwz r0,TI_FLAGS(r9) - andi. r0,r0,_TIF_NEED_RESCHED + andi. r8,r8,_TIF_NEED_RESCHED beq+ restore + lwz r3,_MSR(r1) andi. r0,r3,MSR_EE /* interrupts off? */ beq restore /* don't schedule if so */ #ifdef CONFIG_TRACE_IRQFLAGS @@ -864,8 +903,6 @@ resume_kernel: */ bl trace_hardirqs_on #endif -#else -resume_kernel: #endif /* CONFIG_PREEMPT */ /* interrupts are hard-disabled at this point */ diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index b40e0b4..bdd2dc1 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -593,6 +593,43 @@ _GLOBAL(ret_from_except_lite) b .ret_from_except resume_kernel: + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ + CURRENT_THREAD_INFO(r9, r1) + ld r8,TI_FLAGS(r9) + andis. r8,r8,_TIF_EMULATE_STACK_STORE@h + beq+ 1f + + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */ + + lwz r3,GPR1(r1) + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */ + mr r4,r1 /* src: current exception frame */ + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ + li r6,0 /* start offset: 0 */ + mr r1,r3 /* Reroute the trampoline frame to r1 */ + + /* Copy from the original to the trampoline. */ + li r6,0 + srwi r5,r5,3 + mtctr r5 +2: ldx r0,r6,r4 + stdx r0,r6,r3 + addi r6,r6,8 + bdnz 2b + + /* Do real store operation to complete stwu */ + lwz r5,GPR1(r1) + std r8,0(r5) + + /* Clear _TIF_EMULATE_STACK_STORE flag */ + lis r11,_TIF_EMULATE_STACK_STORE@h + addi r5,r9,TI_FLAGS + ldarx r4,0,r5 + andc r4,r4,r11 + stdcx. r4,0,r5 + bne- 0b +1: + #ifdef CONFIG_PREEMPT /* Check if we need to preempt */ andi. r0,r4,_TIF_NEED_RESCHED
We can't emulate stwu since that may corrupt current exception stack. So we will have to do real store operation in the exception return code. Firstly we'll allocate a trampoline exception frame below the kprobed function stack and copy the current exception frame to the trampoline. Then we can do this real store operation to implement 'stwu', and reroute the trampoline frame to r1 to complete this exception migration. Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com> --- v5: * Simplify copy operation arch/powerpc/kernel/entry_32.S | 49 +++++++++++++++++++++++++++++++++++----- arch/powerpc/kernel/entry_64.S | 37 ++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-)