Hi Nick,

Nicholas Piggin wrote:
This warns and prevents tracing attempted in a real-mode context.

Is this something you're seeing often? Last time we looked at this, KVM was the biggest offender and we introduced paca->ftrace_enabled as a way to disable ftrace while in KVM code.

While this is cheap when handling ftrace_regs_caller() as done in this patch, for simple function tracing (see below), we will have to grab the MSR which will slow things down slightly.


Signed-off-by: Nicholas Piggin <npig...@gmail.com>
---
 arch/powerpc/kernel/trace/ftrace.c            |  3 +++
 .../powerpc/kernel/trace/ftrace_64_mprofile.S | 19 +++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c 
b/arch/powerpc/kernel/trace/ftrace.c
index 7ea0ca044b65..ef965815fcb9 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -949,6 +949,9 @@ unsigned long prepare_ftrace_return(unsigned long parent, 
unsigned long ip,
 {
        unsigned long return_hooker;

+       if (WARN_ON_ONCE((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)))
+               goto out;
+

This is called on function entry to redirect function return to a trampoline if needed. I am not sure if we have (or will have) too many C functions that disable MSR_IR|MSR_DR. Unless the number of such functions is large, it might be preferable to mark specific functions as notrace.

        if (unlikely(ftrace_graph_is_dead()))
                goto out;

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S 
b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index f9fd5f743eba..6205f15cb603 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -51,16 +51,21 @@ _GLOBAL(ftrace_regs_caller)
        SAVE_10GPRS(12, r1)
        SAVE_10GPRS(22, r1)

-       /* Save previous stack pointer (r1) */
-       addi    r8, r1, SWITCH_FRAME_SIZE
-       std     r8, GPR1(r1)
-
        /* Load special regs for save below */
        mfmsr   r8
        mfctr   r9
        mfxer   r10
        mfcr    r11

+       /* Shouldn't be called in real mode */
+       andi.   r3,r8,(MSR_IR|MSR_DR)
+       cmpdi   r3,(MSR_IR|MSR_DR)
+       bne     ftrace_bad_realmode
+
+       /* Save previous stack pointer (r1) */
+       addi    r8, r1, SWITCH_FRAME_SIZE
+       std     r8, GPR1(r1)
+

This stomps on the MSR value in r8, which is saved into pt_regs further below.

You'll also have to handle ftrace_caller() which is used for simple function tracing. We don't read the MSR there today, but that will be needed if we want to suppress tracing.


- Naveen

Reply via email to