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

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

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
> 


  reply	other threads:[~2020-03-23  8:45 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
2020-03-23  8:43     ` Balamuruhan S [this message]
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=bc592c51a195d920a48aaac3c8c42afad76ad488.camel@linux.ibm.com \
    --to=bala24@linux.ibm.com \
    --cc=alistair@popple.id.au \
    --cc=dja@axtens.net \
    --cc=jniethe5@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    /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).