* Re: [PATCH v1 0/5] Implement livepatch on PPC32 [not found] ` <20211213121536.25e5488d@gandalf.local.home> @ 2021-12-13 17:30 ` Christophe Leroy 2021-12-13 17:33 ` Steven Rostedt 0 siblings, 1 reply; 12+ messages in thread From: Christophe Leroy @ 2021-12-13 17:30 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 13/12/2021 à 18:15, Steven Rostedt a écrit : > On Mon, 13 Dec 2021 14:39:15 +0000 > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > >>> Note, you can implement this first, (I looked over the patches and they >>> seem fine) and then update both ppc64 and ppc32 to implement >>> DYNAMIC_FTRACE_WITH_ARGS. >>> >> >> I tried to activate DYNAMIC_FTRACE_WITH_ARGS on PPC32. >> >> I copied into powerpc the changes from 5740a7c71ab6 ("s390/ftrace: add >> HAVE_DYNAMIC_FTRACE_WITH_ARGS support") >> >> Ftrace selftests tell "Testing tracer function_graph: FAILED!". >> >> Is there anything else to do ? > > Yes. Because BPF is now hooking into the function callbacks, it causes > issues with function graph tracer. So what we did was to have function > graph tracing to now use the function tracer callback as well (this allows > both the BPF direct trampolines to work with function graph tracer). > > As it requires DYNAMIC_FTRACE_WITH_ARGS, and x86 was the only one to > support that for now, I decided to make all the archs change function graph > tracing when they implement DYNAMIC_FTRACE_WITH_ARGS too. (It is becoming a > pain to have too many variants of function tracing between the archs). > > The change that did this for x86 was: > > 0c0593b45c9b4 ("x86/ftrace: Make function graph use ftrace directly") > > This actually simplifies the function graph tracer, as you no longer need > it's own entry trampoline (still need the trampoline for the return of the > function). > > What you need to do is: > > In your arch/*/include/asm/ftrace.h add: > > struct ftrace_ops; > > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs); > > > Where ftrace_graph_func() is now what is called for the function graph > tracer, directly from the ftrace callbacks (no longer a secondary > trampoline). > > Define the ftrace_graph_func() to be something like: > > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > struct pt_regs *regs = &fregs->regs; > unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs); > > prepare_ftrace_return(ip, (unsigned long *)stack, 0); > } > > This is called by the function tracer code. But because with > DYNAMIC_FTRACE_WITH_ARGS, we have access to the argument register, we should > also have access to the link register and the stack. Then you can use that > to modify the stack and or link register to jump to the the return > trampoline. > > This should all work with powerpc (both 64 and 32) but if it does not, let > me know. I'm happy to help out. > Thanks, I will try that. I can't find ftrace_graph_func() in s390. Does it mean that s390 doesn't have a working function tracer anymore ? I see your commit 0c0593b45c9b4 ("x86/ftrace: Make function graph use ftrace directly") is dated 8 Oct 2021 while 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") is 4 Oct 2021. Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 17:30 ` [PATCH v1 0/5] Implement livepatch on PPC32 Christophe Leroy @ 2021-12-13 17:33 ` Steven Rostedt 2021-12-13 17:50 ` Christophe Leroy 0 siblings, 1 reply; 12+ messages in thread From: Steven Rostedt @ 2021-12-13 17:33 UTC (permalink / raw) To: Christophe Leroy Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org On Mon, 13 Dec 2021 17:30:48 +0000 Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > Thanks, I will try that. > > I can't find ftrace_graph_func() in s390. Does it mean that s390 doesn't > have a working function tracer anymore ? > > I see your commit 0c0593b45c9b4 ("x86/ftrace: Make function graph use > ftrace directly") is dated 8 Oct 2021 while 5740a7c71ab6 ("s390/ftrace: > add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") is 4 Oct 2021. Hmm, maybe not. I can't test it. This needs to be fixed if that's the case. Thanks for bringing it up! -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 17:33 ` Steven Rostedt @ 2021-12-13 17:50 ` Christophe Leroy 2021-12-13 18:54 ` Steven Rostedt 2021-12-14 14:25 ` Heiko Carstens 0 siblings, 2 replies; 12+ messages in thread From: Christophe Leroy @ 2021-12-13 17:50 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 13/12/2021 à 18:33, Steven Rostedt a écrit : > On Mon, 13 Dec 2021 17:30:48 +0000 > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > >> Thanks, I will try that. >> >> I can't find ftrace_graph_func() in s390. Does it mean that s390 doesn't >> have a working function tracer anymore ? >> >> I see your commit 0c0593b45c9b4 ("x86/ftrace: Make function graph use >> ftrace directly") is dated 8 Oct 2021 while 5740a7c71ab6 ("s390/ftrace: >> add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") is 4 Oct 2021. > > Hmm, maybe not. I can't test it. > > This needs to be fixed if that's the case. > > Thanks for bringing it up! > On PPC32, I did your suggested changes, I get an Oops: [ 8.038441] Testing tracer function_graph: [ 8.064147] Kernel attempted to read user page (4) - exploit attempt? (uid: 0) [ 8.075296] Kernel attempted to read user page (4) - exploit attempt? (uid: 0) [ 8.082424] BUG: Kernel NULL pointer dereference on read at 0x00000004 [ 8.088864] Faulting instruction address: 0xc001468c [ 8.093778] Oops: Kernel access of bad area, sig: 11 [#1] [ 8.099105] BE PAGE_SIZE=16K PREEMPT CMPC885 [ 8.103329] Modules linked in: [ 8.106340] CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty #732 [ 8.115461] NIP: c001468c LR: c00c8414 CTR: c0014674 [ 8.120448] REGS: c902ba00 TRAP: 0300 Not tainted (5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty) [ 8.129398] MSR: 00001032 <ME,IR,DR,RI> CR: 88022252 XER: 20000000 [ 8.135853] DAR: 00000004 DSISR: c0000000 [ 8.135853] GPR00: c00c8414 c902bac0 c2140000 c0015260 c0003ac4 c122db78 00000000 00000300 [ 8.135853] GPR08: c2140000 c0014674 c0015260 00000000 2802b252 00000000 c0004f38 00000000 [ 8.135853] GPR16: 00000000 00000000 00000000 00000000 00000000 00000010 c1037d1c c12d0000 [ 8.135853] GPR24: c121c440 c12b5380 c12b0000 c0003ac4 c0015260 00000000 00000200 c0015260 [ 8.174493] NIP [c001468c] ftrace_graph_func+0x18/0x74 [ 8.179572] LR [c00c8414] arch_ftrace_ops_list_func+0x118/0x230 [ 8.185430] Call Trace: [ 8.187837] [c902bac0] [c12b5380] ftrace_list_end+0x0/0x50 (unreliable) [ 8.194379] [c902bad0] [c00c8414] arch_ftrace_ops_list_func+0x118/0x230 [ 8.200920] [c902bb20] [c001475c] ftrace_call+0x4/0x44 [ 8.205997] [c902bb50] [c0003ac4] DataTLBError_virt+0x114/0x118 [ 8.211848] --- interrupt: 300 at ftrace_graph_func+0x18/0x74 [ 8.217527] NIP: c001468c LR: c00c8414 CTR: c0014674 [ 8.222516] REGS: c902bb60 TRAP: 0300 Not tainted (5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty) [ 8.231466] MSR: 00001032 <ME,IR,DR,RI> CR: 82002842 XER: 20000000 [ 8.237920] DAR: 00000004 DSISR: c0000000 [ 8.237920] GPR00: c00c8414 c902bc20 c2140000 c001573c c001624c c122db78 00000000 00000100 [ 8.237920] GPR08: c2140000 c0014674 c001573c 00000000 22004842 00000000 c0004f38 00000000 [ 8.237920] GPR16: 00000000 00000000 00000000 00000000 00000000 00000010 c1037d1c c12d0000 [ 8.237920] GPR24: c121c440 c12b5380 c12b0000 c001624c c001573c 00000000 00000100 c001573c [ 8.276561] NIP [c001468c] ftrace_graph_func+0x18/0x74 [ 8.281639] LR [c00c8414] arch_ftrace_ops_list_func+0x118/0x230 [ 8.287491] --- interrupt: 300 [ 8.290508] [c902bc20] [00000001] 0x1 (unreliable) [ 8.295242] [c902bc30] [c00c8414] arch_ftrace_ops_list_func+0x118/0x230 [ 8.301782] [c902bc80] [c001475c] ftrace_call+0x4/0x44 [ 8.306860] [c902bcb0] [c001624c] map_kernel_page+0xc8/0x12c [ 8.312454] [c902bd00] [c0019cb0] patch_instruction+0xbc/0x278 [ 8.318221] [c902bd30] [c0013964] ftrace_modify_code+0x38/0xc4 [ 8.323986] [c902bd70] [c00c2c0c] ftrace_replace_code+0x78/0xec [ 8.329838] [c902bd90] [c00c2e30] ftrace_modify_all_code+0xd0/0x148 [ 8.336035] [c902bdb0] [c00c2f38] ftrace_run_update_code+0x28/0x88 [ 8.342145] [c902bdc0] [c00c75dc] ftrace_startup+0x118/0x1e0 [ 8.347739] [c902bde0] [c00e8310] register_ftrace_graph+0x334/0x3c0 [ 8.353935] [c902be20] [c100ccf4] trace_selftest_startup_function_graph+0x64/0x164 [ 8.361422] [c902be50] [c00debc0] run_tracer_selftest+0x120/0x1b4 [ 8.367447] [c902be70] [c100c74c] register_tracer+0x14c/0x218 [ 8.373126] [c902be90] [c0004a30] do_one_initcall+0x44/0x1e8 [ 8.378720] [c902bef0] [c10011f4] kernel_init_freeable+0x1a8/0x250 [ 8.384831] [c902bf20] [c0004f68] kernel_init+0x30/0x150 [ 8.390081] [c902bf30] [c001322c] ret_from_kernel_thread+0x5c/0x64 [ 8.396193] Instruction dump: [ 8.399115] 83c10018 83e1001c 3863489c 7c0803a6 38210020 4e800020 9421fff0 7c0802a6 [ 8.407031] 93e1000c 90010014 93c10008 7c7f1b78 <83c60004> 480d343d 2c030000 40820038 [ 8.415154] ---[ end trace 717d695f81a0970d ]--- I also tried with the additional change below, but still the same: int ftrace_enable_ftrace_graph_caller(void) { return 0; } int ftrace_disable_ftrace_graph_caller(void) { return 0; } Anything else to do ? Full change below: diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index cdac2115eb00..e2b1792b2aae 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -210,6 +210,7 @@ config PPC select HAVE_DEBUG_KMEMLEAK select HAVE_DEBUG_STACKOVERFLOW select HAVE_DYNAMIC_FTRACE + select HAVE_DYNAMIC_FTRACE_WITH_ARGS if MPROFILE_KERNEL || PPC32 select HAVE_DYNAMIC_FTRACE_WITH_REGS if MPROFILE_KERNEL || PPC32 select HAVE_EBPF_JIT select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && POWER7_CPU) diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index debe8c4f7062..68f503294342 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -59,9 +59,24 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) struct dyn_arch_ftrace { struct module *mod; }; + +struct ftrace_regs { + struct pt_regs regs; +}; + +static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) +{ + return &fregs->regs; +} + +struct ftrace_ops; + +#define ftrace_graph_func ftrace_graph_func +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, + struct ftrace_ops *op, struct ftrace_regs *fregs); #endif /* __ASSEMBLY__ */ -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +#if defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS) || defined(CONFIG_DYNAMIC_FTRACE_WITH_ARGS) #define ARCH_SUPPORTS_FTRACE_OPS 1 #endif #endif /* CONFIG_FUNCTION_TRACER */ diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index 80b6285769f2..7662c88c4c0c 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -912,28 +912,12 @@ extern void ftrace_graph_stub(void); int ftrace_enable_ftrace_graph_caller(void) { - unsigned long ip = (unsigned long)(&ftrace_graph_call); - unsigned long addr = (unsigned long)(&ftrace_graph_caller); - unsigned long stub = (unsigned long)(&ftrace_graph_stub); - ppc_inst_t old, new; - - old = ftrace_call_replace(ip, stub, 0); - new = ftrace_call_replace(ip, addr, 0); - - return ftrace_modify_code(ip, old, new); + return 0; } int ftrace_disable_ftrace_graph_caller(void) { - unsigned long ip = (unsigned long)(&ftrace_graph_call); - unsigned long addr = (unsigned long)(&ftrace_graph_caller); - unsigned long stub = (unsigned long)(&ftrace_graph_stub); - ppc_inst_t old, new; - - old = ftrace_call_replace(ip, addr, 0); - new = ftrace_call_replace(ip, stub, 0); - - return ftrace_modify_code(ip, old, new); + return 0; } /* @@ -958,6 +942,12 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, out: return parent; } + +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, + struct ftrace_ops *op, struct ftrace_regs *fregs) +{ + prepare_ftrace_return(ip, kernel_stack_pointer(&fregs->regs), 0); +} #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ #ifdef PPC64_ELF_ABI_v1 Christophe ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 17:50 ` Christophe Leroy @ 2021-12-13 18:54 ` Steven Rostedt 2021-12-13 19:33 ` Christophe Leroy 2021-12-14 14:25 ` Heiko Carstens 1 sibling, 1 reply; 12+ messages in thread From: Steven Rostedt @ 2021-12-13 18:54 UTC (permalink / raw) To: Christophe Leroy Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org On Mon, 13 Dec 2021 17:50:52 +0000 Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > @@ -958,6 +942,12 @@ unsigned long prepare_ftrace_return(unsigned long > parent, unsigned long ip, > out: > return parent; > } > + > +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > + struct ftrace_ops *op, struct ftrace_regs *fregs) > +{ > + prepare_ftrace_return(ip, kernel_stack_pointer(&fregs->regs), 0); > +} I have for powerpc prepare_ftrace_return as: unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp) { unsigned long return_hooker; if (unlikely(ftrace_graph_is_dead())) goto out; if (unlikely(atomic_read(¤t->tracing_graph_pause))) goto out; return_hooker = ppc_function_entry(return_to_handler); if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp)) parent = return_hooker; out: return parent; } Which means you'll need different parameters to it than what x86 has, which has the prototype of: void prepare_ftrace_return(unsigned long ip, unsigned long *parent, unsigned long frame_pointer) and it does not use the frame_pointer for this case, which is why it is zero. For powerpc though, it uses the stack pointer, so you parameters are incorrect. Looks like it should be: prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs)); And that will likely not be enough. I'll need to update the ctr register, as that is where the return address is saved. So you'll probably need it to be: void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { unsigned long parent; parent = prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs)); fregs->regs.ctr = parent; } -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 18:54 ` Steven Rostedt @ 2021-12-13 19:33 ` Christophe Leroy 2021-12-13 19:46 ` Steven Rostedt 0 siblings, 1 reply; 12+ messages in thread From: Christophe Leroy @ 2021-12-13 19:33 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 13/12/2021 à 19:54, Steven Rostedt a écrit : > On Mon, 13 Dec 2021 17:50:52 +0000 > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > >> @@ -958,6 +942,12 @@ unsigned long prepare_ftrace_return(unsigned long >> parent, unsigned long ip, >> out: >> return parent; >> } >> + >> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, >> + struct ftrace_ops *op, struct ftrace_regs *fregs) >> +{ >> + prepare_ftrace_return(ip, kernel_stack_pointer(&fregs->regs), 0); >> +} > > I have for powerpc prepare_ftrace_return as: > > > unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, > unsigned long sp) > { > unsigned long return_hooker; > > if (unlikely(ftrace_graph_is_dead())) > goto out; > > if (unlikely(atomic_read(¤t->tracing_graph_pause))) > goto out; > > return_hooker = ppc_function_entry(return_to_handler); > > if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp)) > parent = return_hooker; > out: > return parent; > } > > Which means you'll need different parameters to it than what x86 has, which > has the prototype of: > > void prepare_ftrace_return(unsigned long ip, unsigned long *parent, > unsigned long frame_pointer) > > and it does not use the frame_pointer for this case, which is why it is > zero. > > For powerpc though, it uses the stack pointer, so you parameters are > incorrect. Looks like it should be: > > prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs)); > > And that will likely not be enough. I'll need to update the ctr register, > as that is where the return address is saved. So you'll probably need it to be: > > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > unsigned long parent; > > parent = prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs)); > fregs->regs.ctr = parent; > } > STill the same Oops, below I will look more closely tomorrow. [ 8.018219] Testing tracer function_graph: [ 8.043884] Kernel attempted to read user page (4) - exploit attempt? (uid: 0) [ 8.055074] Kernel attempted to read user page (4) - exploit attempt? (uid: 0) [ 8.062204] BUG: Kernel NULL pointer dereference on read at 0x00000004 [ 8.068643] Faulting instruction address: 0xc0014694 [ 8.073556] Oops: Kernel access of bad area, sig: 11 [#1] [ 8.078884] BE PAGE_SIZE=16K PREEMPT CMPC885 [ 8.083109] Modules linked in: [ 8.086120] CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty #733 [ 8.095240] NIP: c0014694 LR: c00c8434 CTR: c0014674 [ 8.100227] REGS: c902b9e0 TRAP: 0300 Not tainted (5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty) [ 8.109178] MSR: 00001032 <ME,IR,DR,RI> CR: 88022242 XER: 20000000 [ 8.115632] DAR: 00000004 DSISR: c0000000 [ 8.115632] GPR00: c00c8434 c902baa0 c2140000 c0015278 c0003ac4 c122db78 00000000 00000300 [ 8.115632] GPR08: c2140000 c0014674 c0015278 00000000 2802b242 00000000 c0004f38 00000000 [ 8.115632] GPR16: 00000000 00000000 00000000 00000000 00000000 00000010 c1037d1c c12d0000 [ 8.115632] GPR24: c121c440 c12b5380 c12b0000 c0003ac4 c0015278 00000000 00000000 c122db78 [ 8.154272] NIP [c0014694] ftrace_graph_func+0x20/0x8c [ 8.159351] LR [c00c8434] arch_ftrace_ops_list_func+0x118/0x230 [ 8.165208] Call Trace: [ 8.167616] [c902baa0] [c006c048] vprintk_emit+0x188/0x2a4 (unreliable) [ 8.174158] [c902bac0] [c00c8434] arch_ftrace_ops_list_func+0x118/0x230 [ 8.180699] [c902bb10] [c0014774] ftrace_call+0x4/0x44 [ 8.185776] [c902bb40] [c0003ac4] DataTLBError_virt+0x114/0x118 [ 8.191627] --- interrupt: 300 at ftrace_graph_func+0x20/0x8c [ 8.197306] NIP: c0014694 LR: c00c8434 CTR: c0014674 [ 8.202296] REGS: c902bb50 TRAP: 0300 Not tainted (5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty) [ 8.211245] MSR: 00001032 <ME,IR,DR,RI> CR: 82002842 XER: 20000000 [ 8.217699] DAR: 00000004 DSISR: c0000000 [ 8.217699] GPR00: c00c8434 c902bc10 c2140000 c0015754 c0016264 c122db78 00000000 00000100 [ 8.217699] GPR08: c2140000 c0014674 c0015754 00000000 22004842 00000000 c0004f38 00000000 [ 8.217699] GPR16: 00000000 00000000 00000000 00000000 00000000 00000010 c1037d1c c12d0000 [ 8.217699] GPR24: c121c440 c12b5380 c12b0000 c0016264 c0015754 00000000 00000000 c122db78 [ 8.256340] NIP [c0014694] ftrace_graph_func+0x20/0x8c [ 8.261418] LR [c00c8434] arch_ftrace_ops_list_func+0x118/0x230 [ 8.267270] --- interrupt: 300 [ 8.270288] [c902bc10] [c00adb98] clockevents_program_event+0x108/0x254 (unreliable) [ 8.277947] [c902bc30] [c00c8434] arch_ftrace_ops_list_func+0x118/0x230 [ 8.284488] [c902bc80] [c0014774] ftrace_call+0x4/0x44 [ 8.289565] [c902bcb0] [c0016264] map_kernel_page+0xc8/0x12c [ 8.295159] [c902bd00] [c0019cc8] patch_instruction+0xbc/0x278 [ 8.300926] [c902bd30] [c0013964] ftrace_modify_code+0x38/0xc4 [ 8.306691] [c902bd70] [c00c2c2c] ftrace_replace_code+0x78/0xec [ 8.312543] [c902bd90] [c00c2e50] ftrace_modify_all_code+0xd0/0x148 [ 8.318740] [c902bdb0] [c00c2f58] ftrace_run_update_code+0x28/0x88 [ 8.324850] [c902bdc0] [c00c75fc] ftrace_startup+0x118/0x1e0 [ 8.330443] [c902bde0] [c00e8330] register_ftrace_graph+0x334/0x3c0 [ 8.336640] [c902be20] [c100ccf4] trace_selftest_startup_function_graph+0x64/0x164 [ 8.344127] [c902be50] [c00debe0] run_tracer_selftest+0x120/0x1b4 [ 8.350152] [c902be70] [c100c74c] register_tracer+0x14c/0x218 [ 8.355832] [c902be90] [c0004a30] do_one_initcall+0x44/0x1e8 [ 8.361425] [c902bef0] [c10011f4] kernel_init_freeable+0x1a8/0x250 [ 8.367536] [c902bf20] [c0004f68] kernel_init+0x30/0x150 [ 8.372785] [c902bf30] [c001322c] ret_from_kernel_thread+0x5c/0x64 [ 8.378898] Instruction dump: [ 8.381821] 386348b4 7c0803a6 38210020 4e800020 9421ffe0 7c0802a6 93a10014 93c10018 [ 8.389737] 93e1001c 90010024 93810010 7cde3378 <83860004> 7c7d1b78 7c9f2378 480d344d [ 8.397859] ---[ end trace 93333951fba49ac1 ]--- Thanks Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 19:33 ` Christophe Leroy @ 2021-12-13 19:46 ` Steven Rostedt 2021-12-14 6:09 ` Christophe Leroy 0 siblings, 1 reply; 12+ messages in thread From: Steven Rostedt @ 2021-12-13 19:46 UTC (permalink / raw) To: Christophe Leroy Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org On Mon, 13 Dec 2021 19:33:47 +0000 Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > STill the same Oops, below Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug this. > I will look more closely tomorrow. OK, thanks. -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 19:46 ` Steven Rostedt @ 2021-12-14 6:09 ` Christophe Leroy 2021-12-14 7:35 ` Christophe Leroy 0 siblings, 1 reply; 12+ messages in thread From: Christophe Leroy @ 2021-12-14 6:09 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 13/12/2021 à 20:46, Steven Rostedt a écrit : > On Mon, 13 Dec 2021 19:33:47 +0000 > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > >> STill the same Oops, below > > Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug > this. > > >> I will look more closely tomorrow. > > OK, thanks. > The Oops was due to ftrace_caller() setting the regs argument to NULL. After fixing that, I'm back into a situation where I get "Testing tracer function_graph: FAILED!" Will continue investigating. Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-14 6:09 ` Christophe Leroy @ 2021-12-14 7:35 ` Christophe Leroy 2021-12-14 14:01 ` Steven Rostedt 0 siblings, 1 reply; 12+ messages in thread From: Christophe Leroy @ 2021-12-14 7:35 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 14/12/2021 à 07:09, Christophe Leroy a écrit : > > > Le 13/12/2021 à 20:46, Steven Rostedt a écrit : >> On Mon, 13 Dec 2021 19:33:47 +0000 >> Christophe Leroy <christophe.leroy@csgroup.eu> wrote: >> >>> STill the same Oops, below >> >> Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug >> this. >> >> >>> I will look more closely tomorrow. >> >> OK, thanks. >> > > The Oops was due to ftrace_caller() setting the regs argument to NULL. > > After fixing that, I'm back into a situation where I get "Testing tracer > function_graph: FAILED!" > > Will continue investigating. > trace_selftest_startup_function_graph() calls register_ftrace_direct() which returns -ENOSUPP because powerpc doesn't select CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS. Should TEST_DIRECT_TRAMP depend on CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS ? Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-14 7:35 ` Christophe Leroy @ 2021-12-14 14:01 ` Steven Rostedt 2021-12-18 16:12 ` Christophe Leroy 0 siblings, 1 reply; 12+ messages in thread From: Steven Rostedt @ 2021-12-14 14:01 UTC (permalink / raw) To: Christophe Leroy Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org On Tue, 14 Dec 2021 08:35:14 +0100 Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > Will continue investigating. > > > > trace_selftest_startup_function_graph() calls register_ftrace_direct() > which returns -ENOSUPP because powerpc doesn't select > CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS. > > Should TEST_DIRECT_TRAMP depend on CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS ? Yes, that should be: #if defined(CONFIG_DYNAMIC_FTRACE) && \ defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) #define TEST_DIRECT_TRAMP noinline __noclone static void trace_direct_tramp(void) { } #endif And make it test it with or without the args. Thanks for finding this. -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-14 14:01 ` Steven Rostedt @ 2021-12-18 16:12 ` Christophe Leroy 0 siblings, 0 replies; 12+ messages in thread From: Christophe Leroy @ 2021-12-18 16:12 UTC (permalink / raw) To: Steven Rostedt Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 14/12/2021 à 15:01, Steven Rostedt a écrit : > On Tue, 14 Dec 2021 08:35:14 +0100 > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > >>> Will continue investigating. >>> >> >> trace_selftest_startup_function_graph() calls register_ftrace_direct() >> which returns -ENOSUPP because powerpc doesn't select >> CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS. >> >> Should TEST_DIRECT_TRAMP depend on CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS ? > > Yes, that should be: > > #if defined(CONFIG_DYNAMIC_FTRACE) && \ > defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) > #define TEST_DIRECT_TRAMP > noinline __noclone static void trace_direct_tramp(void) { } > #endif > > > And make it test it with or without the args. > Shouldn't it just be: #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS Because register_ftrace_direct() depends on that symbol, so if you have CONFIG_DYNAMIC_FTRACE && CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS but not DYNAMIC_FTRACE_WITH_REGS then CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is unset and register_ftrace_direct() returns -ENOTSUPP Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-13 17:50 ` Christophe Leroy 2021-12-13 18:54 ` Steven Rostedt @ 2021-12-14 14:25 ` Heiko Carstens 2021-12-14 15:12 ` Christophe Leroy 1 sibling, 1 reply; 12+ messages in thread From: Heiko Carstens @ 2021-12-14 14:25 UTC (permalink / raw) To: Christophe Leroy Cc: Steven Rostedt, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org On Mon, Dec 13, 2021 at 05:50:52PM +0000, Christophe Leroy wrote: > Le 13/12/2021 à 18:33, Steven Rostedt a écrit : > > On Mon, 13 Dec 2021 17:30:48 +0000 > > Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > >> Thanks, I will try that. > >> > >> I can't find ftrace_graph_func() in s390. Does it mean that s390 doesn't > >> have a working function tracer anymore ? > >> > >> I see your commit 0c0593b45c9b4 ("x86/ftrace: Make function graph use > >> ftrace directly") is dated 8 Oct 2021 while 5740a7c71ab6 ("s390/ftrace: > >> add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") is 4 Oct 2021. > > > > Hmm, maybe not. I can't test it. > > > > This needs to be fixed if that's the case. > > > > Thanks for bringing it up! It still works, we run the full ftrace/kprobes selftests from the kernel every day on multiple machines with several kernels (besides other Linus' tree, but also linux-next). That said, I wanted to change s390's code follow what x86 is currently doing anyway. One thing to note: commit 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") looks only that simple because ftrace_caller _and_ ftrace_regs_caller used to save all register contents into the pt_regs structure, which never was a requirement, but implicitly fulfills the HAVE_DYNAMIC_FTRACE_WITH_ARGS requirements. Not sure if powerpc passes enough register contents via pt_regs for HAVE_DYNAMIC_FTRACE_WITH_ARGS though. Might be something to check? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Implement livepatch on PPC32 2021-12-14 14:25 ` Heiko Carstens @ 2021-12-14 15:12 ` Christophe Leroy 0 siblings, 0 replies; 12+ messages in thread From: Christophe Leroy @ 2021-12-14 15:12 UTC (permalink / raw) To: Heiko Carstens Cc: Steven Rostedt, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, Ingo Molnar, Naveen N . Rao, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, live-patching@vger.kernel.org, linux-s390@vger.kernel.org Le 14/12/2021 à 15:25, Heiko Carstens a écrit : > On Mon, Dec 13, 2021 at 05:50:52PM +0000, Christophe Leroy wrote: >> Le 13/12/2021 à 18:33, Steven Rostedt a écrit : >>> On Mon, 13 Dec 2021 17:30:48 +0000 >>> Christophe Leroy <christophe.leroy@csgroup.eu> wrote: >>> >>>> Thanks, I will try that. >>>> >>>> I can't find ftrace_graph_func() in s390. Does it mean that s390 doesn't >>>> have a working function tracer anymore ? >>>> >>>> I see your commit 0c0593b45c9b4 ("x86/ftrace: Make function graph use >>>> ftrace directly") is dated 8 Oct 2021 while 5740a7c71ab6 ("s390/ftrace: >>>> add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") is 4 Oct 2021. >>> >>> Hmm, maybe not. I can't test it. >>> >>> This needs to be fixed if that's the case. >>> >>> Thanks for bringing it up! > > It still works, we run the full ftrace/kprobes selftests from the > kernel every day on multiple machines with several kernels (besides > other Linus' tree, but also linux-next). That said, I wanted to change > s390's code follow what x86 is currently doing anyway. > > One thing to note: commit 5740a7c71ab6 ("s390/ftrace: add > HAVE_DYNAMIC_FTRACE_WITH_ARGS support") looks only that simple because > ftrace_caller _and_ ftrace_regs_caller used to save all register > contents into the pt_regs structure, which never was a requirement, > but implicitly fulfills the HAVE_DYNAMIC_FTRACE_WITH_ARGS > requirements. > Not sure if powerpc passes enough register contents via pt_regs for > HAVE_DYNAMIC_FTRACE_WITH_ARGS though. Might be something to check? > In fact there is no need to rework the function graph logic. It still works as is with HAVE_DYNAMIC_FTRACE_WITH_ARGS. The problem was that the sefltests were failing with CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS not being selected on powerpc. As s390 selects CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS, there is no problem. Thanks Christophe ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-18 16:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1635423081.git.christophe.leroy@csgroup.eu>
[not found] ` <20211028093547.48c69dfe@gandalf.local.home>
[not found] ` <6209682d-0caa-b779-8763-376a984d8ed8@csgroup.eu>
[not found] ` <20211213121536.25e5488d@gandalf.local.home>
2021-12-13 17:30 ` [PATCH v1 0/5] Implement livepatch on PPC32 Christophe Leroy
2021-12-13 17:33 ` Steven Rostedt
2021-12-13 17:50 ` Christophe Leroy
2021-12-13 18:54 ` Steven Rostedt
2021-12-13 19:33 ` Christophe Leroy
2021-12-13 19:46 ` Steven Rostedt
2021-12-14 6:09 ` Christophe Leroy
2021-12-14 7:35 ` Christophe Leroy
2021-12-14 14:01 ` Steven Rostedt
2021-12-18 16:12 ` Christophe Leroy
2021-12-14 14:25 ` Heiko Carstens
2021-12-14 15:12 ` Christophe Leroy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox