On 2017/04/27 08:19PM, Michael Ellerman wrote:

"Naveen N. Rao" [off-list ref] writes:

quoted

It is actually safe to probe system_call() in entry_64.S, but only till
.Lsyscall_exit. To allow this, convert .Lsyscall_exit to a non-local
symbol __system_call() and blacklist that symbol, rather than
system_call().

I'm not sure I like this. The reason we made it a local symbol in the
first place is because it made backtraces look odd:

  commit 4c3b21686111e0ac6018469dacbc5549f9915cf8
  Author:     Michael Ellerman [off-list ref]
  AuthorDate: Fri Dec 5 21:16:59 2014 +1100
  
  powerpc/kernel: Make syscall_exit a local label
  
  Currently when we back trace something that is in a syscall we see
  something like this:
  
  [c000000000000000] [c000000000000000] SyS_read+0x6c/0x110
  [c000000000000000] [c000000000000000] syscall_exit+0x0/0x98
  
  Although it's entirely correct, seeing syscall_exit at the bottom can be
  confusing - we were exiting from a syscall and then called SyS_read() ?
  
  If we instead change syscall_exit to be a local label we get something
  more intuitive:
  
  [c0000001fa46fde0] [c00000000026719c] SyS_read+0x6c/0x110
  [c0000001fa46fe30] [c000000000009264] system_call+0x38/0xd0
  
  ie. we were handling a system call, and it was SyS_read().


I think you know that, although you didn't mention it in the change log,
because you've called the new symbol __system_call. But that is not a
great name either because that's not what it does.

Yes, you're right. I used __system_call since I felt that it won't cause 
confusion like syscall_exit did. I agree it's not a great name, but we 
need _some_ label other than system_call if we want to allow probing at 
this point.

Also, if I'm reading this right, there is no other place to probe if we 
want to capture all system call entries.

So, I felt this would be good to have.

quoted

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 380361c0bb6a..e030ce34dd66 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S

@@ -176,7 +176,7 @@ system_call:			/* label this so stack traces look sane */
 	mtctr   r12
 	bctrl			/* Call handler */
 
-.Lsyscall_exit:
+__system_call:
 	std	r3,RESULT(r1)
 	CURRENT_THREAD_INFO(r12, r1)

  
Why can't we kprobe the std and the rotate to current thread info?

Is the real no-probe point just here, prior to the clearing of MSR_RI ?

	ld	r8,_MSR(r1)
#ifdef CONFIG_PPC_BOOK3S
	/* No MSR:RI on BookE */

We can probe at all those places, just not once MSR_RI is unset. So, the 
no-probe point is just *after* the mtmsrd.

However, for kprobe blacklisting, the granularity is at a function level 
(or ASM labels). As such, we will have to blacklist all of 
syscall_exit/__system_call.


Regards,
Naveen