[PATCH v4 09/16] powerpc: Use a function for reading instructions

Nicholas Piggin npiggin at gmail.com
Mon Mar 23 19:00:05 AEDT 2020


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 <jniethe5 at 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.

> 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



More information about the Linuxppc-dev mailing list