Re: [PATCH v4 09/16] powerpc: Use a function for reading instructions
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2020-03-23 08:10:36
Jordan Niethe's on March 20, 2020 3:18 pm:
quoted hunk ↗ jump to hunk
Prefixed instructions will mean there are instructions of different length. As a result dereferencing a pointer to an instruction will not necessarily give the desired result. Introduce a function for reading instructions from memory into the instruction data type. Signed-off-by: Jordan Niethe <redacted> --- v4: New to series --- arch/powerpc/include/asm/uprobes.h | 4 ++-- arch/powerpc/kernel/kprobes.c | 8 ++++---- arch/powerpc/kernel/mce_power.c | 2 +- arch/powerpc/kernel/optprobes.c | 6 +++--- arch/powerpc/kernel/trace/ftrace.c | 33 +++++++++++++++++++----------- arch/powerpc/kernel/uprobes.c | 2 +- arch/powerpc/lib/code-patching.c | 22 ++++++++++---------- arch/powerpc/lib/feature-fixups.c | 6 +++--- arch/powerpc/xmon/xmon.c | 6 +++--- 9 files changed, 49 insertions(+), 40 deletions(-)diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h index 2bbdf27d09b5..fff3c5fc90b5 100644 --- a/arch/powerpc/include/asm/uprobes.h +++ b/arch/powerpc/include/asm/uprobes.h@@ -23,8 +23,8 @@ typedef ppc_opcode_t uprobe_opcode_t; struct arch_uprobe { union { - u32 insn; - u32 ixol; + u8 insn[MAX_UINSN_BYTES]; + u8 ixol[MAX_UINSN_BYTES]; }; };
Hmm. I wonder if this should be a different patch. Not sure if raw bytes is a good idea here. ppc probes also has a ppc_opcode_t, maybe could be replaced with ppc_insn_t and used here instead? Also can't find where you define ppc_inst_read.
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index 7614a9f537fd..ad451205f268 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c@@ -41,6 +41,12 @@ #define NUM_FTRACE_TRAMPS 8 static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS]; +static long +read_inst(ppc_inst *inst, const void *src) +{ + return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE); +}
Humbly suggest probe_kernel_inst_read. Thanks, Nick