linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [v3][PATCH 1/3] powerpc/kprobe: introduce a new thread flag
@ 2012-09-11  2:20 Tiejun Chen
  2012-09-11  2:20 ` [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame Tiejun Chen
  2012-09-11  2:20 ` [v3][PATCH 3/3] ppc/kprobe: don't emulate store when kprobe stwu r1 Tiejun Chen
  0 siblings, 2 replies; 10+ messages in thread
From: Tiejun Chen @ 2012-09-11  2:20 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

We need to add a new thread flag, TIF_EMULATE_STACK_STORE,
for emulating stack store operation while exiting exception.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
v3:
* rebase on next
* restore those clobbered registers
* add for 64-bit
* retest with kprobe do_fork()/show_interrupts()
	for fsl-p4080 and fsl-p5020, separately

 arch/powerpc/include/asm/thread_info.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index e942203..8ceea14 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -104,6 +104,8 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
 #define TIF_UPROBE		14	/* breakpointed or single-stepping */
 #define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
+#define TIF_EMULATE_STACK_STORE	16	/* Is an instruction emulation
+						for stack store? */
 
 /* as above, but as bit values */
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
@@ -121,6 +123,7 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_UPROBE		(1<<TIF_UPROBE)
 #define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
+#define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
 #define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-11  2:20 [v3][PATCH 1/3] powerpc/kprobe: introduce a new thread flag Tiejun Chen
@ 2012-09-11  2:20 ` Tiejun Chen
  2012-09-11  5:51   ` Benjamin Herrenschmidt
  2012-09-11  2:20 ` [v3][PATCH 3/3] ppc/kprobe: don't emulate store when kprobe stwu r1 Tiejun Chen
  1 sibling, 1 reply; 10+ messages in thread
From: Tiejun Chen @ 2012-09-11  2:20 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

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>
---
 arch/powerpc/kernel/entry_32.S |   45 ++++++++++++++++++++++++++++++++++------
 arch/powerpc/kernel/entry_64.S |   32 ++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index ead5016..6cfe12f 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -831,19 +831,54 @@ 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, _TIF_EMULATE_STACK_STORE */
+	CURRENT_THREAD_INFO(r9, r1)
+	lwz	r0,TI_FLAGS(r9)
+	andis.	r0,r0,_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 */
+	mr	r1,r3			/* Reroute the trampoline frame to r1 */
+	bl	memcpy			/* Copy from the original to the trampoline */
+
+	CURRENT_THREAD_INFO(r9, r1)
+	lwz	r0,TI_FLAGS(r9)		/* Restore this clobbered r0 */
+
+	/* Do real store operation to complete stwu */
+	lwz	r5,GPR1(r1)
+	stw	r8,0(r5)
+
+	/* Clear _TIF_EMULATE_STACK_STORE flag */
+	CURRENT_THREAD_INFO(r9, r1)
+	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 */
 	CURRENT_THREAD_INFO(r9, r1)
-	lwz	r0,TI_PREEMPT(r9)
-	cmpwi	0,r0,0		/* if non-zero, just restore regs and return */
+	lwz	r8,TI_PREEMPT(r9)
+	cmpwi	0,r8,0		/* if non-zero, just restore regs and return */
 	bne	restore
-	lwz	r0,TI_FLAGS(r9)
 	andi.	r0,r0,_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 +899,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..b6d7483 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -593,6 +593,38 @@ _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	r0,TI_FLAGS(r9)
+	andis.	r0,r0,_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 */
+	mr	r1,r3			/* Reroute the trampoline frame to r1 */
+	bl	memcpy			/* Copy from the original to the trampoline */
+
+	CURRENT_THREAD_INFO(r9, r1)
+	ld	r4,TI_FLAGS(r9)		/* Restore this clobbered r4 */
+
+	/* Do real store operation to complete stwu */
+	lwz	r5,GPR1(r1)
+	std	r8,0(r5)
+
+	/* Clear _TIF_EMULATE_STACK_STORE flag */
+	CURRENT_THREAD_INFO(r9, r1)
+	lis	r11,_TIF_EMULATE_STACK_STORE@h
+	addi	r5,r9,TI_FLAGS
+0:	ldarx	r8,0,r5
+	andc	r8,r8,r11
+	stdcx.	r8,0,r5
+	bne-	0b
+1:
+
 #ifdef CONFIG_PREEMPT
 	/* Check if we need to preempt */
 	andi.	r0,r4,_TIF_NEED_RESCHED
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [v3][PATCH 3/3] ppc/kprobe: don't emulate store when kprobe stwu r1
  2012-09-11  2:20 [v3][PATCH 1/3] powerpc/kprobe: introduce a new thread flag Tiejun Chen
  2012-09-11  2:20 ` [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame Tiejun Chen
@ 2012-09-11  2:20 ` Tiejun Chen
  1 sibling, 0 replies; 10+ messages in thread
From: Tiejun Chen @ 2012-09-11  2:20 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

We don't do the real store operation for kprobing 'stwu Rx,(y)R1'
since this may corrupt the exception frame, now we will do this
operation safely in exception return code after migrate current
exception frame below the kprobed function stack.

So we only update gpr[1] here and trigger a thread flag to mask
this.

Note we should make sure if we trigger kernel stack over flow.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
 arch/powerpc/lib/sstep.c |   36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 9a52349..e15c521 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -566,7 +566,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
 	unsigned long int ea;
 	unsigned int cr, mb, me, sh;
 	int err;
-	unsigned long old_ra;
+	unsigned long old_ra, val3;
 	long ival;
 
 	opcode = instr >> 26;
@@ -1486,11 +1486,43 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
 		goto ldst_done;
 
 	case 36:	/* stw */
-	case 37:	/* stwu */
 		val = regs->gpr[rd];
 		err = write_mem(val, dform_ea(instr, regs), 4, regs);
 		goto ldst_done;
 
+	case 37:	/* stwu */
+		val = regs->gpr[rd];
+		val3 = dform_ea(instr, regs);
+		/*
+		 * For PPC32 we always use stwu to change stack point with r1. So
+		 * this emulated store may corrupt the exception frame, now we
+		 * have to provide the exception frame trampoline, which is pushed
+		 * below the kprobed function stack. So we only update gpr[1] but
+		 * don't emulate the real store operation. We will do real store
+		 * operation safely in exception return code by checking this flag.
+		 */
+		if ((ra == 1) && !(regs->msr & MSR_PR) \
+			&& (val3 >= (regs->gpr[1] - STACK_INT_FRAME_SIZE))) {
+			/*
+			 * Check if we will touch kernel sack overflow
+			 */
+			if (val3 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {
+				printk(KERN_CRIT "Can't kprobe this since Kernel stack overflow.\n");
+				err = -EINVAL;
+				break;
+			}
+
+			/*
+			 * Check if we already set since that means we'll
+			 * lose the previous value.
+			 */
+			WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
+			set_thread_flag(TIF_EMULATE_STACK_STORE);
+			err = 0;
+		} else
+			err = write_mem(val, val3, 4, regs);
+		goto ldst_done;
+
 	case 38:	/* stb */
 	case 39:	/* stbu */
 		val = regs->gpr[rd];
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-11  2:20 ` [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame Tiejun Chen
@ 2012-09-11  5:51   ` Benjamin Herrenschmidt
  2012-09-12  8:38     ` tiejun.chen
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-11  5:51 UTC (permalink / raw)
  To: Tiejun Chen; +Cc: linuxppc-dev

On Tue, 2012-09-11 at 10:20 +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.

Ok, so not quite there yet :-)

See below:

> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>  arch/powerpc/kernel/entry_32.S |   45 ++++++++++++++++++++++++++++++++++------
>  arch/powerpc/kernel/entry_64.S |   32 ++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index ead5016..6cfe12f 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -831,19 +831,54 @@ 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, _TIF_EMULATE_STACK_STORE */
> +	CURRENT_THREAD_INFO(r9, r1)
> +	lwz	r0,TI_FLAGS(r9)
> +	andis.	r0,r0,_TIF_EMULATE_STACK_STORE@h
> +	beq+	1f

So you used r0 to load the TI_FLAGS and immediately clobbered it in
andis. forcing you to re-load them later down. Instead, put them in r8

	lwz	r8,TI_FLAGS(r9)
	andis.	r0,r8,_TIF_*
	beq+	*

> +	addi	r8,r1,INT_FRAME_SIZE	/* Get the kprobed function entry */

Then you put your entry in r8 ....

> +	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 */

Which you just clobbered... oops :-)

So you need to store that old r1 somewhere fist then retrieve it
after the memcpy call. That or open-code the memcpy to avoid all
the clobbering problems.

> +	CURRENT_THREAD_INFO(r9, r1)
> +	lwz	r0,TI_FLAGS(r9)		/* Restore this clobbered r0 */

Re-load in r8 as suggested above ? Anyway, it doesn't matter you don't
actually need to load it at all because you re-load it in your
lwarx/stwcx. loop further down

> +	/* Do real store operation to complete stwu */
> +	lwz	r5,GPR1(r1)
> +	stw	r8,0(r5)

(Storing a clobbered value.)

> +	/* Clear _TIF_EMULATE_STACK_STORE flag */
> +	CURRENT_THREAD_INFO(r9, r1)

Why re-calculate r9 here ? you just did 4 lines above

> +	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

So here, r8 contains TI_FLAGS

> +1:

And if you do the change I suggested above, here too.

> +#ifdef CONFIG_PREEMPT
>  	/* check current_thread_info->preempt_count */
>  	CURRENT_THREAD_INFO(r9, r1)

r9 already has what you want

> -	lwz	r0,TI_PREEMPT(r9)
> -	cmpwi	0,r0,0		/* if non-zero, just restore regs and return */
> +	lwz	r8,TI_PREEMPT(r9)
> +	cmpwi	0,r8,0		/* if non-zero, just restore regs and return */

Leave that to be r0, r8 has your TI_FLAGS already.

>  	bne	restore
> -	lwz	r0,TI_FLAGS(r9)

See above.

>  	andi.	r0,r0,_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 +899,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..b6d7483 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -593,6 +593,38 @@ _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	r0,TI_FLAGS(r9)
> +	andis.	r0,r0,_TIF_EMULATE_STACK_STORE@h
> +	beq+	1f

Similar comments to 32-bit

> +	addi	r8,r1,INT_FRAME_SIZE	/* Get the kprobed function entry */

That gets clobbered too.

> +	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 */
> +
> +	CURRENT_THREAD_INFO(r9, r1)
> +	ld	r4,TI_FLAGS(r9)		/* Restore this clobbered r4 */

Usueless reloads

> +	/* Do real store operation to complete stwu */
> +	lwz	r5,GPR1(r1)
> +	std	r8,0(r5)
> +
> +	/* Clear _TIF_EMULATE_STACK_STORE flag */
> +	CURRENT_THREAD_INFO(r9, r1)
> +	lis	r11,_TIF_EMULATE_STACK_STORE@h
> +	addi	r5,r9,TI_FLAGS
> +0:	ldarx	r8,0,r5
> +	andc	r8,r8,r11
> +	stdcx.	r8,0,r5
> +	bne-	0b
> +1:
> +
>  #ifdef CONFIG_PREEMPT
>  	/* Check if we need to preempt */
>  	andi.	r0,r4,_TIF_NEED_RESCHED

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-11  5:51   ` Benjamin Herrenschmidt
@ 2012-09-12  8:38     ` tiejun.chen
  2012-09-12  8:43       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 10+ messages in thread
From: tiejun.chen @ 2012-09-12  8:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

On 09/11/2012 01:51 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2012-09-11 at 10:20 +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.
>
> Ok, so not quite there yet :-)
>
> See below:
>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>>   arch/powerpc/kernel/entry_32.S |   45 ++++++++++++++++++++++++++++++++++------
>>   arch/powerpc/kernel/entry_64.S |   32 ++++++++++++++++++++++++++++
>>   2 files changed, 71 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index ead5016..6cfe12f 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -831,19 +831,54 @@ 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, _TIF_EMULATE_STACK_STORE */
>> +	CURRENT_THREAD_INFO(r9, r1)
>> +	lwz	r0,TI_FLAGS(r9)
>> +	andis.	r0,r0,_TIF_EMULATE_STACK_STORE@h
>> +	beq+	1f
>
> So you used r0 to load the TI_FLAGS and immediately clobbered it in
> andis. forcing you to re-load them later down. Instead, put them in r8
>
> 	lwz	r8,TI_FLAGS(r9)
> 	andis.	r0,r8,_TIF_*
> 	beq+	*
>
>> +	addi	r8,r1,INT_FRAME_SIZE	/* Get the kprobed function entry */
>
> Then you put your entry in r8 ....

I'll update this for 32b and 64b sections.

>
>> +	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 */
>
> Which you just clobbered... oops :-)
>
> So you need to store that old r1 somewhere fist then retrieve it
> after the memcpy call. That or open-code the memcpy to avoid all
> the clobbering problems.

Maybe we can use copy_and_flush() since looks copy_and_flush() only clobber r0, 
r6 and LR explicitly.

I'll resync these comments for v4.

Tiejun

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-12  8:38     ` tiejun.chen
@ 2012-09-12  8:43       ` Benjamin Herrenschmidt
  2012-09-12  8:55         ` tiejun.chen
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-12  8:43 UTC (permalink / raw)
  To: tiejun.chen; +Cc: linuxppc-dev

On Wed, 2012-09-12 at 16:38 +0800, tiejun.chen wrote:
> > So you need to store that old r1 somewhere fist then retrieve it
> > after the memcpy call. That or open-code the memcpy to avoid all
> > the clobbering problems.
> 
> Maybe we can use copy_and_flush() since looks copy_and_flush() only
> clobber r0, 
> r6 and LR explicitly.
> 
> I'll resync these comments for v4. 

I'd say just open code it. You already have src and dst in registers,
the length can easily be put in ctr... easier that way, not clobbering
to worry about nor stack frame to create etc...

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-12  8:43       ` Benjamin Herrenschmidt
@ 2012-09-12  8:55         ` tiejun.chen
  2012-09-12 10:38           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 10+ messages in thread
From: tiejun.chen @ 2012-09-12  8:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

On 09/12/2012 04:43 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-09-12 at 16:38 +0800, tiejun.chen wrote:
>>> So you need to store that old r1 somewhere fist then retrieve it
>>> after the memcpy call. That or open-code the memcpy to avoid all
>>> the clobbering problems.
>>
>> Maybe we can use copy_and_flush() since looks copy_and_flush() only
>> clobber r0,
>> r6 and LR explicitly.
>>
>> I'll resync these comments for v4.
>
> I'd say just open code it. You already have src and dst in registers,
> the length can easily be put in ctr... easier that way, not clobbering

ctr should be easier :)

> to worry about nor stack frame to create etc...

If you don't like this v4, let me know and then I can go back memcpy for next 
version.

Tiejun

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-12  8:55         ` tiejun.chen
@ 2012-09-12 10:38           ` Benjamin Herrenschmidt
  2012-09-13 10:36             ` tiejun.chen
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-12 10:38 UTC (permalink / raw)
  To: tiejun.chen; +Cc: linuxppc-dev

On Wed, 2012-09-12 at 16:55 +0800, tiejun.chen wrote:
> > to worry about nor stack frame to create etc...
> 
> If you don't like this v4, let me know and then I can go back memcpy
> for next 
> version. 

Just open code the whole copy. It should be easy really. As I said, you
have the src and dst already in registers and you know they are aligned,
so just put the size of the frame in a register (divided by 4), do an
mtctr and do a little load_update/store_update loop to do the copy, all
in the asm.

Ben.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-12 10:38           ` Benjamin Herrenschmidt
@ 2012-09-13 10:36             ` tiejun.chen
  2012-09-13 21:42               ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 10+ messages in thread
From: tiejun.chen @ 2012-09-13 10:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

On 09/12/2012 06:38 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-09-12 at 16:55 +0800, tiejun.chen wrote:
>>> to worry about nor stack frame to create etc...
>>
>> If you don't like this v4, let me know and then I can go back memcpy
>> for next
>> version.
>
> Just open code the whole copy. It should be easy really. As I said, you
> have the src and dst already in registers and you know they are aligned,
> so just put the size of the frame in a register (divided by 4), do an
> mtctr and do a little load_update/store_update loop to do the copy, all
> in the asm.

Is the following Okay?

---
  arch/powerpc/kernel/entry_32.S |   55 +++++++++++++++++++++++++++++++++++-----
  arch/powerpc/kernel/entry_64.S |   45 ++++++++++++++++++++++++++++++++
  2 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index ead5016..3b56bba 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -32,6 +32,7 @@
  #include <asm/unistd.h>
  #include <asm/ftrace.h>
  #include <asm/ptrace.h>
+#include <asm/cache.h>

  #undef SHOW_SYSCALLS
  #undef SHOW_SYSCALLS_TASK
@@ -831,19 +832,63 @@ 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. */
+	addi	r5,r5,-4
+	addi	r6,r6,-4
+4:	li	r0,L1_CACHE_BYTES/4
+	mtctr	r0
+3:	addi	r6,r6,4			/* copy a cache line */
+	lwzx	r0,r6,r4
+	stwx	r0,r6,r3
+	bdnz	3b
+	dcbst	r6,r3			/* write it to memory */
+	sync
+	cmplw	0,r6,r5
+	blt	4b
+
+	/* 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 +909,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..cc43b64 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -593,6 +593,51 @@ _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. */
+	addi	r5,r5,-8
+	addi	r6,r6,-8
+4:	li	r0,8
+	mtctr	r0	
+3:	addi	r6,r6,8			/* copy a cache line		*/
+	ldx	r0,r6,r4
+	stdx	r0,r6,r3
+	bdnz	3b
+	dcbst	r6,r3			/* write it to memory		*/
+	sync
+	cmpld	0,r6,r5
+	blt	4b
+	sync
+
+	bl	.copy_and_flush
+
+	/* 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
-- 
1.7.9.5

Tiejun

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
  2012-09-13 10:36             ` tiejun.chen
@ 2012-09-13 21:42               ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-13 21:42 UTC (permalink / raw)
  To: tiejun.chen; +Cc: linuxppc-dev

On Thu, 2012-09-13 at 18:36 +0800, tiejun.chen wrote:
> On 09/12/2012 06:38 PM, Benjamin Herrenschmidt wrote:
> > On Wed, 2012-09-12 at 16:55 +0800, tiejun.chen wrote:
> >>> to worry about nor stack frame to create etc...
> >>
> >> If you don't like this v4, let me know and then I can go back memcpy
> >> for next
> >> version.
> >
> > Just open code the whole copy. It should be easy really. As I said, you
> > have the src and dst already in registers and you know they are aligned,
> > so just put the size of the frame in a register (divided by 4), do an
> > mtctr and do a little load_update/store_update loop to do the copy, all
> > in the asm.
> 
> Is the following Okay?

Well, why did you bother with the flushes ? One of the main reason I
wasn't too happy with hijacking copy_and_flush is that ... you really
don't need to bother about flushing the cache :-) The flush in that
routine is about copying kernel code around and making sure the I/D
caches stay in sync.

Cheers,
Ben.

> ---
>   arch/powerpc/kernel/entry_32.S |   55 +++++++++++++++++++++++++++++++++++-----
>   arch/powerpc/kernel/entry_64.S |   45 ++++++++++++++++++++++++++++++++
>   2 files changed, 94 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index ead5016..3b56bba 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -32,6 +32,7 @@
>   #include <asm/unistd.h>
>   #include <asm/ftrace.h>
>   #include <asm/ptrace.h>
> +#include <asm/cache.h>
> 
>   #undef SHOW_SYSCALLS
>   #undef SHOW_SYSCALLS_TASK
> @@ -831,19 +832,63 @@ 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. */
> +	addi	r5,r5,-4
> +	addi	r6,r6,-4
> +4:	li	r0,L1_CACHE_BYTES/4
> +	mtctr	r0
> +3:	addi	r6,r6,4			/* copy a cache line */
> +	lwzx	r0,r6,r4
> +	stwx	r0,r6,r3
> +	bdnz	3b
> +	dcbst	r6,r3			/* write it to memory */
> +	sync
> +	cmplw	0,r6,r5
> +	blt	4b
> +
> +	/* 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 +909,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..cc43b64 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -593,6 +593,51 @@ _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. */
> +	addi	r5,r5,-8
> +	addi	r6,r6,-8
> +4:	li	r0,8
> +	mtctr	r0	
> +3:	addi	r6,r6,8			/* copy a cache line		*/
> +	ldx	r0,r6,r4
> +	stdx	r0,r6,r3
> +	bdnz	3b
> +	dcbst	r6,r3			/* write it to memory		*/
> +	sync
> +	cmpld	0,r6,r5
> +	blt	4b
> +	sync
> +
> +	bl	.copy_and_flush
> +
> +	/* 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-09-13 21:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-11  2:20 [v3][PATCH 1/3] powerpc/kprobe: introduce a new thread flag Tiejun Chen
2012-09-11  2:20 ` [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame Tiejun Chen
2012-09-11  5:51   ` Benjamin Herrenschmidt
2012-09-12  8:38     ` tiejun.chen
2012-09-12  8:43       ` Benjamin Herrenschmidt
2012-09-12  8:55         ` tiejun.chen
2012-09-12 10:38           ` Benjamin Herrenschmidt
2012-09-13 10:36             ` tiejun.chen
2012-09-13 21:42               ` Benjamin Herrenschmidt
2012-09-11  2:20 ` [v3][PATCH 3/3] ppc/kprobe: don't emulate store when kprobe stwu r1 Tiejun Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).