linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Jordan Niethe <jniethe5@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc: alistair@popple.id.au, dja@axtens.net, bala24@linux.ibm.com
Subject: Re: [PATCH v4 09/16] powerpc: Use a function for reading instructions
Date: Mon, 23 Mar 2020 18:00:05 +1000	[thread overview]
Message-ID: <1584946118.tw98vo7hqx.astroid@bobo.none> (raw)
In-Reply-To: <20200320051809.24332-10-jniethe5@gmail.com>

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@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


  reply	other threads:[~2020-03-23  8:10 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-20  5:17 [PATCH v4 00/16] Initial Prefixed Instruction support Jordan Niethe
2020-03-20  5:17 ` [PATCH v4 01/16] powerpc/xmon: Remove store_inst() for patch_instruction() Jordan Niethe
2020-03-23  6:19   ` Nicholas Piggin
2020-03-20  5:17 ` [PATCH v4 02/16] xmon: Move out-of-line instructions to text section Jordan Niethe
2020-03-23  5:59   ` Balamuruhan S
2020-03-23  6:05   ` Balamuruhan S
2020-03-23  9:26     ` Jordan Niethe
2020-03-23  6:22   ` Nicholas Piggin
2020-03-20  5:17 ` [PATCH v4 03/16] powerpc: Use a datatype for instructions Jordan Niethe
2020-03-23  6:07   ` Balamuruhan S
2020-03-23  6:23   ` Nicholas Piggin
2020-03-23  9:28     ` Jordan Niethe
2020-03-23  9:51       ` Nicholas Piggin
2020-03-24  2:58         ` Michael Ellerman
2020-03-24  3:21           ` Jordan Niethe
2020-04-01 10:32   ` Balamuruhan S
2020-04-01 23:52     ` Jordan Niethe
2020-04-02 23:44       ` Alistair Popple
2020-04-03  0:09         ` Jordan Niethe
2020-03-20  5:17 ` [PATCH v4 04/16] powerpc: Use a macro for creating instructions from u32s Jordan Niethe
2020-03-23  6:26   ` Nicholas Piggin
2020-03-23  9:29     ` Jordan Niethe
2020-03-20  5:17 ` [PATCH v4 05/16] powerpc: Use a function for masking instructions Jordan Niethe
2020-03-23  6:37   ` Nicholas Piggin
2020-03-23  9:31     ` Jordan Niethe
2020-03-20  5:17 ` [PATCH v4 06/16] powerpc: Use a function for getting the instruction op code Jordan Niethe
2020-03-23  6:54   ` Balamuruhan S
2020-03-23  9:35     ` Jordan Niethe
2020-03-20  5:18 ` [PATCH v4 07/16] powerpc: Introduce functions for instruction nullity and equality Jordan Niethe
2020-03-23  6:43   ` Nicholas Piggin
2020-03-23  9:31     ` Jordan Niethe
2020-03-20  5:18 ` [PATCH v4 08/16] powerpc: Use an accessor for word instructions Jordan Niethe
2020-03-23 11:12   ` Balamuruhan S
2020-03-24  3:18     ` Jordan Niethe
2020-03-24  6:22       ` Balamuruhan S
2020-03-20  5:18 ` [PATCH v4 09/16] powerpc: Use a function for reading instructions Jordan Niethe
2020-03-23  8:00   ` Nicholas Piggin [this message]
2020-03-23  8:43     ` Balamuruhan S
2020-03-23 10:09     ` Jordan Niethe
2020-03-23 10:36       ` Nicholas Piggin
2020-03-20  5:18 ` [PATCH v4 10/16] powerpc: Make test_translate_branch() independent of instruction length Jordan Niethe
2020-03-20  5:18 ` [PATCH v4 11/16] powerpc: Enable Prefixed Instructions Jordan Niethe
2020-03-23  7:02   ` Nicholas Piggin
2020-03-20  5:18 ` [PATCH v4 12/16] powerpc: Define new SRR1 bits for a future ISA version Jordan Niethe
2020-03-23  7:03   ` Nicholas Piggin
2020-03-20  5:18 ` [PATCH v4 13/16] powerpc: Support prefixed instructions in alignment handler Jordan Niethe
2020-03-23  7:05   ` Nicholas Piggin
2020-03-23  9:35     ` Jordan Niethe
2020-03-20  5:18 ` [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type Jordan Niethe
2020-03-23  6:58   ` Nicholas Piggin
2020-03-23  7:33   ` Nicholas Piggin
2020-03-23 23:45     ` Jordan Niethe
2020-03-24  5:40       ` Nicholas Piggin
2020-03-30  9:05   ` Alistair Popple
2020-03-30  9:13     ` Jordan Niethe
2020-03-20  5:18 ` [PATCH v4 15/16] powerpc sstep: Add support for prefixed load/stores Jordan Niethe
2020-03-23  8:54   ` Balamuruhan S
2020-03-20  5:18 ` [PATCH v4 16/16] powerpc sstep: Add support for prefixed fixed-point arithmetic Jordan Niethe
2020-03-23 10:05   ` Balamuruhan S
2020-03-23  6:18 ` [PATCH v4 00/16] Initial Prefixed Instruction support Nicholas Piggin
2020-03-23  9:25   ` Jordan Niethe
2020-03-23 10:17     ` Nicholas Piggin
2020-03-24  2:54       ` Jordan Niethe
2020-03-24  2:59         ` Jordan Niethe
2020-03-24  5:44         ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1584946118.tw98vo7hqx.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=alistair@popple.id.au \
    --cc=bala24@linux.ibm.com \
    --cc=dja@axtens.net \
    --cc=jniethe5@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).