Patchwork [RFC] powerpc: Emulate nop too

login
register
mail settings
Submitter Ananth N Mavinakayanahalli
Date May 27, 2010, 2:12 p.m.
Message ID <20100527141203.GA20770@in.ibm.com>
Download mbox | patch
Permalink /patch/53759/
State Rejected
Headers show

Comments

Ananth N Mavinakayanahalli - May 27, 2010, 2:12 p.m.
Hi Paul,

While we are at it, can we also add nop to the list of emulated
instructions?

Ananth
---
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Emulate ori 0,0,0 (nop).

The long winded way is to do:

        case 24:
                rd = (instr >> 21) & 0x1f;
                if (rd != 0)
                        break;
                rb = (instr >> 11) & 0x1f;
                if (rb != 0)
                        break;
                imm = (instr & 0xffff);
                if (imm == 0) {
                        regs->nip += 4;
                        return 1;
                }
                break;

But, the following is more straightforward for this case at least.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 arch/powerpc/lib/sstep.c |    6 ++++++
 1 file changed, 6 insertions(+)
Kumar Gala - May 27, 2010, 8:22 p.m.
On May 27, 2010, at 9:12 AM, Ananth N Mavinakayanahalli wrote:

> Hi Paul,
> 
> While we are at it, can we also add nop to the list of emulated
> instructions?

Dare I ask why we need to emulate nop?

- k

> 
> Ananth
> ---
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Emulate ori 0,0,0 (nop).
> 
> The long winded way is to do:
> 
>        case 24:
>                rd = (instr >> 21) & 0x1f;
>                if (rd != 0)
>                        break;
>                rb = (instr >> 11) & 0x1f;
>                if (rb != 0)
>                        break;
>                imm = (instr & 0xffff);
>                if (imm == 0) {
>                        regs->nip += 4;
>                        return 1;
>                }
>                break;
> 
> But, the following is more straightforward for this case at least.
> 
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/lib/sstep.c |    6 ++++++
> 1 file changed, 6 insertions(+)
> 
> Index: linux-27may/arch/powerpc/lib/sstep.c
> ===================================================================
> --- linux-27may.orig/arch/powerpc/lib/sstep.c
> +++ linux-27may/arch/powerpc/lib/sstep.c
> @@ -412,6 +412,12 @@ int __kprobes emulate_step(struct pt_reg
> 	int err;
> 	mm_segment_t oldfs;
> 
> +	/* ori 0,0,0 is a nop. Emulate that too */
> +	if (instr == 0x60000000) {
> +		regs->nip += 4;
> +		return 1;
> +	}
> +
> 	opcode = instr >> 26;
> 	switch (opcode) {
> 	case 16:	/* bc */
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Paul Mackerras - May 28, 2010, 2:05 a.m.
On Thu, May 27, 2010 at 07:42:03PM +0530, Ananth N Mavinakayanahalli wrote:

> While we are at it, can we also add nop to the list of emulated
> instructions?

I have a patch in development that emulates most of the arithmetic,
logical and shift/rotate instructions, including ori.

While you're here (in a virtual sense at least :), could you explain
what's going on with the emulate_step() call in resume_execution() in
arch/powerpc/kernel/kprobes.c?  It looks like, having decided that
emulate_step() can't handle the instruction, you single-step the
instruction out of line and then call emulate_step again on the same
instruction, in resume_execution().  Why on earth is it trying to
emulate the instruction when it has already been executed at this
point?  Is there any reason why we can't just remove the emulate_step
call from resume_execution()?

Paul.
Michael Neuling - May 28, 2010, 2:28 a.m.
In message <20100527141203.GA20770@in.ibm.com> you wrote:
> Hi Paul,
> 
> While we are at it, can we also add nop to the list of emulated
> instructions?
> 
> Ananth
> ---
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Emulate ori 0,0,0 (nop).
> 
> The long winded way is to do:
> 
>         case 24:
>                 rd = (instr >> 21) & 0x1f;
>                 if (rd != 0)
>                         break;
>                 rb = (instr >> 11) & 0x1f;
>                 if (rb != 0)
>                         break;

Don't we just need rb == rd?

>                 imm = (instr & 0xffff);
>                 if (imm == 0) {
>                         regs->nip += 4;
>                         return 1;
>                 }
>                 break;
> 
> But, the following is more straightforward for this case at least.
> 
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/lib/sstep.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> Index: linux-27may/arch/powerpc/lib/sstep.c
> ===================================================================
> --- linux-27may.orig/arch/powerpc/lib/sstep.c
> +++ linux-27may/arch/powerpc/lib/sstep.c
> @@ -412,6 +412,12 @@ int __kprobes emulate_step(struct pt_reg
>  	int err;
>  	mm_segment_t oldfs;
>  
> +	/* ori 0,0,0 is a nop. Emulate that too */
> +	if (instr == 0x60000000) {
> +		regs->nip += 4;
> +		return 1;
> +	}
> +
>  	opcode = instr >> 26;
>  	switch (opcode) {
>  	case 16:	/* bc */
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Ananth N Mavinakayanahalli - May 28, 2010, 3:52 a.m.
On Thu, May 27, 2010 at 03:22:45PM -0500, Kumar Gala wrote:
> 
> On May 27, 2010, at 9:12 AM, Ananth N Mavinakayanahalli wrote:
> 
> > Hi Paul,
> > 
> > While we are at it, can we also add nop to the list of emulated
> > instructions?
> 
> Dare I ask why we need to emulate nop?

We are close to getting userspace probes done. Some userspace apps
that've gained 'markers' from DTrace or elsewhere, are being enabled to
expose those in through debuginfo on Linux and the underlying instruction
there is a nop.

Ananth
Ananth N Mavinakayanahalli - May 28, 2010, 4:16 a.m.
On Fri, May 28, 2010 at 12:28:43PM +1000, Michael Neuling wrote:
> 
> 
> In message <20100527141203.GA20770@in.ibm.com> you wrote:
> > Hi Paul,
> > 
> > While we are at it, can we also add nop to the list of emulated
> > instructions?
> > 
> > Ananth
> > ---
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > Emulate ori 0,0,0 (nop).
> > 
> > The long winded way is to do:
> > 
> >         case 24:
> >                 rd = (instr >> 21) & 0x1f;
> >                 if (rd != 0)
> >                         break;
> >                 rb = (instr >> 11) & 0x1f;
> >                 if (rb != 0)
> >                         break;
> 
> Don't we just need rb == rd?

Sure. But for this case, just checking against the opcode seems simple
enough.

Ananth
Michael Neuling - May 28, 2010, 4:23 a.m.
In message <20100528041645.GB25946@in.ibm.com> you wrote:
> On Fri, May 28, 2010 at 12:28:43PM +1000, Michael Neuling wrote:
> > 
> > 
> > In message <20100527141203.GA20770@in.ibm.com> you wrote:
> > > Hi Paul,
> > > 
> > > While we are at it, can we also add nop to the list of emulated
> > > instructions?
> > > 
> > > Ananth
> > > ---
> > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > 
> > > Emulate ori 0,0,0 (nop).
> > > 
> > > The long winded way is to do:
> > > 
> > >         case 24:
> > >                 rd = (instr >> 21) & 0x1f;
> > >                 if (rd != 0)
> > >                         break;
> > >                 rb = (instr >> 11) & 0x1f;
> > >                 if (rb != 0)
> > >                         break;
> > 
> > Don't we just need rb == rd?
> 
> Sure. But for this case, just checking against the opcode seems simple
> enough.

Simple, sure.  You could not emulate anything and remove the code
altogether.  That would be truly simple. :-P

Why not eliminate as much as possible?

Anyway, sounds like paulus as a better solution.

Mikey
Ananth N Mavinakayanahalli - May 28, 2010, 5:54 a.m.
On Fri, May 28, 2010 at 02:23:30PM +1000, Michael Neuling wrote:
> 
> 
> In message <20100528041645.GB25946@in.ibm.com> you wrote:
> > On Fri, May 28, 2010 at 12:28:43PM +1000, Michael Neuling wrote:
> > > 
> > > 
> > > In message <20100527141203.GA20770@in.ibm.com> you wrote:
> > > > Hi Paul,
> > > > 
> > > > While we are at it, can we also add nop to the list of emulated
> > > > instructions?
> > > > 
> > > > Ananth
> > > > ---
> > > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > > 
> > > > Emulate ori 0,0,0 (nop).
> > > > 
> > > > The long winded way is to do:
> > > > 
> > > >         case 24:
> > > >                 rd = (instr >> 21) & 0x1f;
> > > >                 if (rd != 0)
> > > >                         break;
> > > >                 rb = (instr >> 11) & 0x1f;
> > > >                 if (rb != 0)
> > > >                         break;
> > > 
> > > Don't we just need rb == rd?
> > 
> > Sure. But for this case, just checking against the opcode seems simple
> > enough.
> 
> Simple, sure.  You could not emulate anything and remove the code
> altogether.  That would be truly simple. :-P

Ah.. I misunderstood your initial point about rb == rd with imm = 0.

> Why not eliminate as much as possible?
> 
> Anyway, sounds like paulus as a better solution.

Right.

Ananth

Patch

Index: linux-27may/arch/powerpc/lib/sstep.c
===================================================================
--- linux-27may.orig/arch/powerpc/lib/sstep.c
+++ linux-27may/arch/powerpc/lib/sstep.c
@@ -412,6 +412,12 @@  int __kprobes emulate_step(struct pt_reg
 	int err;
 	mm_segment_t oldfs;
 
+	/* ori 0,0,0 is a nop. Emulate that too */
+	if (instr == 0x60000000) {
+		regs->nip += 4;
+		return 1;
+	}
+
 	opcode = instr >> 26;
 	switch (opcode) {
 	case 16:	/* bc */