Re: [RFC][PATCH 3/3] x86/ftrace: Use text_poke()

2019-08-26 Thread Peter Zijlstra
On Mon, Aug 26, 2019 at 02:51:41PM +0200, Peter Zijlstra wrote:
> Move ftrace over to using the generic x86 text_poke functions; this
> avoids having a second/different copy of that code around.
> 
> Cc: Daniel Bristot de Oliveira 
> Cc: Steven Rostedt 
> Signed-off-by: Peter Zijlstra (Intel) 

*sigh*.. one of those days again. I seem to have tested without this
patch applied.

Let me go fix this.


[RFC][PATCH 3/3] x86/ftrace: Use text_poke()

2019-08-26 Thread Peter Zijlstra
Move ftrace over to using the generic x86 text_poke functions; this
avoids having a second/different copy of that code around.

Cc: Daniel Bristot de Oliveira 
Cc: Steven Rostedt 
Signed-off-by: Peter Zijlstra (Intel) 
---
 arch/x86/include/asm/ftrace.h |2 
 arch/x86/kernel/ftrace.c  |  571 +++---
 arch/x86/kernel/traps.c   |9 
 3 files changed, 51 insertions(+), 531 deletions(-)

--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -35,8 +35,6 @@ struct dyn_arch_ftrace {
/* No extra data needed for x86 */
 };
 
-int ftrace_int3_handler(struct pt_regs *regs);
-
 #define FTRACE_GRAPH_TRAMP_ADDR FTRACE_GRAPH_ADDR
 
 #endif /*  CONFIG_DYNAMIC_FTRACE */
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -43,16 +43,17 @@ int ftrace_arch_code_modify_prepare(void
 * ftrace has it set to "read/write".
 */
mutex_lock(_mutex);
-   set_kernel_text_rw();
-   set_all_modules_text_rw();
+   WARN_ON_ONCE(tp_vec_nr);
return 0;
 }
 
 int ftrace_arch_code_modify_post_process(void)
 __releases(_mutex)
 {
-   set_all_modules_text_ro();
-   set_kernel_text_ro();
+   if (tp_vec_nr) {
+   text_poke_bp_batch(tp_vec, tp_vec_nr);
+   tp_vec_nr = 0;
+   }
mutex_unlock(_mutex);
return 0;
 }
@@ -60,67 +61,36 @@ int ftrace_arch_code_modify_post_process
 union ftrace_code_union {
char code[MCOUNT_INSN_SIZE];
struct {
-   unsigned char op;
+   char op;
int offset;
} __attribute__((packed));
 };
 
-static int ftrace_calc_offset(long ip, long addr)
-{
-   return (int)(addr - ip);
-}
-
-static unsigned char *
-ftrace_text_replace(unsigned char op, unsigned long ip, unsigned long addr)
+static char *ftrace_text_replace(char op, unsigned long ip, unsigned long addr)
 {
static union ftrace_code_union calc;
 
-   calc.op = op;
-   calc.offset = ftrace_calc_offset(ip + MCOUNT_INSN_SIZE, addr);
+   calc.op = op;
+   calc.offset = (int)(addr - (ip + MCOUNT_INSN_SIZE));
 
return calc.code;
 }
 
-static unsigned char *
-ftrace_call_replace(unsigned long ip, unsigned long addr)
-{
-   return ftrace_text_replace(0xe8, ip, addr);
-}
-
-static inline int
-within(unsigned long addr, unsigned long start, unsigned long end)
+static char *ftrace_nop_replace(void)
 {
-   return addr >= start && addr < end;
-}
-
-static unsigned long text_ip_addr(unsigned long ip)
-{
-   /*
-* On x86_64, kernel text mappings are mapped read-only, so we use
-* the kernel identity mapping instead of the kernel text mapping
-* to modify the kernel text.
-*
-* For 32bit kernels, these mappings are same and we can use
-* kernel identity mapping to modify code.
-*/
-   if (within(ip, (unsigned long)_text, (unsigned long)_etext))
-   ip = (unsigned long)__va(__pa_symbol(ip));
-
-   return ip;
+   return ideal_nops[NOP_ATOMIC5];
 }
 
-static const unsigned char *ftrace_nop_replace(void)
+static char *ftrace_call_replace(unsigned long ip, unsigned long addr)
 {
-   return ideal_nops[NOP_ATOMIC5];
+   return ftrace_text_replace(CALL_INSN_OPCODE, ip, addr);
 }
 
-static int
-ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
-  unsigned const char *new_code)
+static int;
+ftrace_modify_code_direct(unsigned long ip, const char *old_code,
+ const char *new_code)
 {
-   unsigned char replaced[MCOUNT_INSN_SIZE];
-
-   ftrace_expected = old_code;
+   char cur_code[MCOUNT_INSN_SIZE];
 
/*
 * Note:
@@ -129,31 +99,23 @@ ftrace_modify_code_direct(unsigned long
 * Carefully read and modify the code with probe_kernel_*(), and make
 * sure what we read is what we expected it to be before modifying it.
 */
-
/* read the text we want to modify */
-   if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
+   if (probe_kernel_read(cur_code, (void *)ip, MCOUNT_INSN_SIZE))
return -EFAULT;
 
/* Make sure it is what we expect it to be */
-   if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
+   if (memcmp(cur_code, old_code, MCOUNT_INSN_SIZE) != 0)
return -EINVAL;
 
-   ip = text_ip_addr(ip);
-
/* replace the text with the new text */
-   if (probe_kernel_write((void *)ip, new_code, MCOUNT_INSN_SIZE))
-   return -EPERM;
-
-   sync_core();
-
+   text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
return 0;
 }
 
-int ftrace_make_nop(struct module *mod,
-   struct dyn_ftrace *rec, unsigned long addr)
+int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long 
addr)
 {
-   unsigned const char *new, *old;
unsigned long