On Thu, 2011-12-15 at 19:00 +0800, Tiejun Chen wrote:
> 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.c...@windriver.com>
> ---
> arch/powerpc/kernel/entry_32.S | 35 +++++++++++++++++++++++++++++++++++
> 1 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 56212bc..0cdd27d 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -850,6 +850,41 @@ resume_kernel:
>
> /* interrupts are hard-disabled at this point */
> restore:
> + lwz r3,_MSR(r1) /* Returning to user mode? */
> + andi. r0,r3,MSR_PR
> + bne 1f
.../...
Wouldn't it be better to use resume_kernel here ?
IE. We already have restore_user vs. resume_kernel labels, including
the PR test. In the !PREEMPT case, resume_kernel is empty but it's
there, so you can just "populate" it, ie, something like:
restore_user:
... existing dbcr0 stuff ...
b restore
resume_kernel: <-- removed the ifdef CONFIG_PREEMPT
... Do the stack store business here...
#ifdef CONFIG_PREEMPT
... move the preempt code here...
#endif
restore:
... existing stuff ...
Also, the added advantage is that the code to calc
the thread info pointer and load the TI_FLAG can be
shared now between your stuff and preempt, ie:
resume_kernel:
rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
lwz r0,TI_FLAGS(r9)
andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
bne- emulate_stack_store
#ifdef CONFIG_PREEMPT
lwz r8,TI_PREEMPT(r9) <-- note use of r8 instead of r0,
I -think- r8 can be clobbered
here but pls dbl check
cmpwi 0,r8,0
bne restore
andi. r0,r0,_TIF_NEED_RESCHED
etc...
> + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */
> + rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
> + lwz r0,TI_FLAGS(r9)
> + andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
> + beq+ 1f
> +
> + addi r9,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 */
> + mr r1,r3 /* Reroute the trampoline frame to r1 */
> + bl memcpy /* Copy from the original to the
> trampoline */
> +
> + /* Do real store operation to complete stwu */
> + lwz r5,GPR1(r1)
> + stw r9,0(r5)
Ok, I think I -finally- understand your trick of using r1 +
INT_FRAME_SIZE as the value to store :-) It makes sense and it's
actually a nice hack :-)
I would recommend that in the C code part of the emulation, you
add some sanity checking to make sure we don't overflow the
kernel stack etc... it should come in generally handy especially
if what's your trying to debug with kprobes is a kernel stack
overflow :-)
> + /* Clear _TIF_EMULATE_STACK_STORE flag */
> + rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
> + lis r11,_TIF_EMULATE_STACK_STORE@h
> + addi r9,r9,TI_FLAGS
> +0: lwarx r8,0,r9
> + andc r8,r8,r11
> +#ifdef CONFIG_IBM405_ERR77
> + dcbt 0,r9
> +#endif
> + stwcx. r8,0,r9
> + bne- 0b
> +1:
> +
> #ifdef CONFIG_44x
> BEGIN_MMU_FTR_SECTION
> b 1f
BTW. Are you going to do a ppc64 variant of that patch ?
Cheers,
Ben.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
