Commit Message

On Fri, May 28, 2010 at 12:05:56PM +1000, Paul Mackerras wrote:
> 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.
OK.
> 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()?
You are right. We needed emulate_step() in resume_execution() before we
had the code to handle the emulation in kprobe_handler() at the time of
the breakpoint it. At the time we needed it mainly to ensure branch
targets are reflected correctly in regs->nip if the stepped instruction
was a branch.
However, we now don't get to post_kprobe_handler() at all if
emulate_step() returned 1 at the time of the breakpoint hit. It suffices
if we just fixup the nip here. Patch below. Tested for various
instructions that can and can't be emulated...
---
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
emulate_step() in kprobe_handler() would've already determined if the
probed instruction can be emulated. We single-step in hardware only if
the instruction couldn't be emulated. resume_execution() therefore is
superfluous -- all we need is to fix up the instruction pointer after
single-stepping.
Thanks to Paul Mackerras for catching this.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
---
arch/powerpc/kernel/kprobes.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)