On Mon, 2020-03-23 at 18:00 +1000, Nicholas Piggin wrote:
> Jordan Niethe's on March 20, 2020 3:18 pm:
> > 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 <jniet...@gmail.com>
> > ---
> > 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.
Nick, ppc_inst_read and macro PPC_INST you have asked in patch 4 are defined in
asm/inst.h with patch 3 (powerpc: Use a datatype for instructions)
-- Bala
>
> > 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
>
