From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-06 09:20:06
From: Ananth N Mavinakayanahalli <redacted>
On RISC architectures like powerpc, instructions are fixed size.
Instruction analysis on such platforms is just a matter of (insn % 4).
Pass the vaddr at which the uprobe is to be inserted so that
arch_uprobe_analyze_insn() can flag misaligned registration requests.
Signed-off-by: Ananth N Mavinakaynahalli <redacted>
---
arch/x86/include/asm/uprobes.h | 2 +-
arch/x86/kernel/uprobes.c | 3 ++-
kernel/events/uprobes.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
Index: uprobes-24may/arch/x86/include/asm/uprobes.h
===================================================================
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-06 09:22:12
From: Ananth N Mavinakayanahalli <redacted>
This is the port of uprobes to powerpc. Usage is similar to x86.
One TODO in this port compared to x86 is the uprobe abort_xol() logic.
x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
if a signal was caused when the uprobed instruction was single-stepped/
emulated, in which case, we reset the instruction pointer to the probed
address and retry the probe again.
[root@xxxx ~]# ./bin/perf probe -x /lib64/libc.so.6 malloc
Added new event:
probe_libc:malloc (on 0xb4860)
You can now use it in all perf tools, such as:
perf record -e probe_libc:malloc -aR sleep 1
[root@xxxx ~]# ./bin/perf record -e probe_libc:malloc -aR sleep 20
[ perf record: Woken up 22 times to write data ]
[ perf record: Captured and wrote 5.843 MB perf.data (~255302 samples) ]
[root@xxxx ~]# ./bin/perf report --stdio
# ========
# captured on: Mon Jun 4 05:26:31 2012
# hostname : xxxx.ibm.com
# os release : 3.4.0-uprobe
# perf version : 3.4.0
# arch : ppc64
# nrcpus online : 4
# nrcpus avail : 4
# cpudesc : POWER6 (raw), altivec supported
# cpuid : 62,769
# total memory : 7310528 kB
# cmdline : /root/bin/perf record -e probe_libc:malloc -aR sleep 20
# event : name = probe_libc:malloc, type = 2, config = 0x124, config1 = 0x0, con
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# ========
#
# Samples: 83K of event 'probe_libc:malloc'
# Event count (approx.): 83484
#
# Overhead Command Shared Object Symbol
# ........ ............ ............. ..........
#
69.05% tar libc-2.12.so [.] malloc
28.57% rm libc-2.12.so [.] malloc
1.32% avahi-daemon libc-2.12.so [.] malloc
0.58% bash libc-2.12.so [.] malloc
0.28% sshd libc-2.12.so [.] malloc
0.08% irqbalance libc-2.12.so [.] malloc
0.05% bzip2 libc-2.12.so [.] malloc
0.04% sleep libc-2.12.so [.] malloc
0.03% multipathd libc-2.12.so [.] malloc
0.01% sendmail libc-2.12.so [.] malloc
0.01% automount libc-2.12.so [.] malloc
Signed-off-by: Ananth N Mavinakayanahalli <redacted>
Index: linux-3.5-rc1/arch/powerpc/include/asm/thread_info.h
===================================================================
@@ -0,0 +1,163 @@+/*+*User-spaceProbes(UProbes)forpowerpc+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*+*YoushouldhavereceivedacopyoftheGNUGeneralPublicLicense+*alongwiththisprogram;ifnot,writetotheFreeSoftware+*Foundation,Inc.,59TemplePlace-Suite330,Boston,MA02111-1307,USA.+*+*Copyright(C)IBMCorporation,2007-2012+*+*Adaptedfromthex86portbyAnanthNMavinakayanahalli<ananth@in.ibm.com>+*/+#include<linux/kernel.h>+#include<linux/sched.h>+#include<linux/ptrace.h>+#include<linux/uprobes.h>+#include<linux/uaccess.h>++#include<linux/kdebug.h>+#include<asm/sstep.h>++/**+*arch_uprobe_analyze_insn+*@mm:theprobedaddressspace.+*@arch_uprobe:theprobepointinformation.+*Return0onsuccessora-venumberonerror.+*/+intarch_uprobe_analyze_insn(structarch_uprobe*auprobe,structmm_struct*mm,loff_tvaddr)+{+if(vaddr&0x03)+return-EINVAL;+return0;+}++/*+*arch_uprobe_pre_xol-preparetoexecuteoutofline.+*@auprobe:theprobepointinformation.+*@regs:reflectsthesaveduserstateofcurrenttask.+*/+intarch_uprobe_pre_xol(structarch_uprobe*auprobe,structpt_regs*regs)+{+/* FIXME: We don't support abort_xol on powerpc for now */+regs->nip=current->utask->xol_vaddr;+return0;+}++/**+*uprobe_get_swbp_addr-computeaddressofswbpgivenpost-swbpregs+*@regs:Reflectsthesavedstateofthetaskafterithashitabreakpoint+*instruction.+*Returntheaddressofthebreakpointinstruction.+*/+unsignedlonguprobe_get_swbp_addr(structpt_regs*regs)+{+returninstruction_pointer(regs);+}++/*+*Ifxolinsnitselftrapsandgeneratesasignal(SIGILL/SIGSEGV/etc),+*thendetectthecasewhereasinglesteppedinstructionjumpsbacktoits+*ownaddress.Itisassumedthatanythinglikedo_page_fault/do_trap/etc+*setsthread.trap_nr!=-1.+*+*FIXME:powerpchoweverdoesn'thavethread.trap_nryet.+*+*arch_uprobe_pre_xol/arch_uprobe_post_xolsave/restorethread.trap_nr,+*arch_uprobe_xol_was_trapped()simplychecksthat->trap_nrisnotequalto+*UPROBE_TRAP_NR==-1setbyarch_uprobe_pre_xol().+*/+boolarch_uprobe_xol_was_trapped(structtask_struct*t)+{+/* FIXME: We don't support abort_xol on powerpc for now */+returnfalse;+}++/*+*Calledaftersingle-stepping.ToavoidtheSMPproblemsthatcan+*occurwhenwetemporarilyputbacktheoriginalopcodeto+*single-step,wesingle-steppedacopyoftheinstruction.+*+*Thisfunctionpreparestoresumeexecutionafterthesingle-step.+*/+intarch_uprobe_post_xol(structarch_uprobe*auprobe,structpt_regs*regs)+{+/* FIXME: We don't support abort_xol on powerpc for now */++/*+*Onpowerpc,exceptforloadsandstores,mostinstructions+*includingonesthataltercodeflow(branches,calls,returns)+*areemulatedinthekernel.Wegethereonlyiftheemulation+*supportdoesn'texistandhavetofix-upthenextinstruction+*tobeexecuted.+*/+regs->nip=current->utask->vaddr+MAX_UINSN_BYTES;+return0;+}++/* callback routine for handling exceptions. */+intarch_uprobe_exception_notify(structnotifier_block*self,unsignedlongval,void*data)+{+structdie_args*args=data;+structpt_regs*regs=args->regs;+intret=NOTIFY_DONE;++/* We are only interested in userspace traps */+if(regs&&!user_mode(regs))+returnNOTIFY_DONE;++switch(val){+caseDIE_BPT:+if(uprobe_pre_sstep_notifier(regs))+ret=NOTIFY_STOP;+break;+caseDIE_SSTEP:+if(uprobe_post_sstep_notifier(regs))+ret=NOTIFY_STOP;+default:+break;+}+returnret;+}++/*+*ThisfunctiongetscalledwhenXOLinstructioneithergetstrappedor+*thethreadhasafatalsignal,soresettheinstructionpointertoits+*probedaddress.+*/+voidarch_uprobe_abort_xol(structarch_uprobe*auprobe,structpt_regs*regs)+{+/* FIXME: We don't support abort_xol on powerpc for now */+return;+}++/*+*Seeiftheinstructioncanbeemulated.+*Returnstrueifinstructionwasemulated,falseotherwise.+*/+boolarch_uprobe_skip_sstep(structarch_uprobe*auprobe,structpt_regs*regs)+{+intret;+unsignedintinsn;++memcpy(&insn,auprobe->insn,MAX_UINSN_BYTES);++/*+*emulate_step()returns1iftheinsnwassuccessfullyemulated.+*Forallothercases,weneedtosingle-stepinhardware.+*/+ret=emulate_step(regs,insn);+if(ret>0)+returntrue;++returnfalse;+}
From: Peter Zijlstra <peterz@infradead.org> Date: 2012-06-06 09:27:33
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
One TODO in this port compared to x86 is the uprobe abort_xol() logic.
x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
if a signal was caused when the uprobed instruction was single-stepped/
emulated, in which case, we reset the instruction pointer to the probed
address and retry the probe again.=20
Another curious difference is that x86 uses an instruction decoder and
contains massive tables to validate we can probe a particular
instruction.
Can we probe all possible PPC instructions?
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-06 09:35:53
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
quoted
One TODO in this port compared to x86 is the uprobe abort_xol() logic.
x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
if a signal was caused when the uprobed instruction was single-stepped/
emulated, in which case, we reset the instruction pointer to the probed
address and retry the probe again.
Another curious difference is that x86 uses an instruction decoder and
contains massive tables to validate we can probe a particular
instruction.
Can we probe all possible PPC instructions?
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
I am not aware of specific caveats for vector/altivec instructions;
maybe Paul or Ben are more suitable to comment on that.
Ananth
Don't we traditionally use unsigned long to pass vaddrs?
Right. But the vaddr we pass here is vma_info->vaddr which is loff_t.
I guess I should've made that clear in the patch description.
Why not fix struct vma_info's vaddr type?
Calculating and comparing vaddr results either uses variables of type loff_t.
To avoid typecasting and avoid overflow at each of these places, we used
loff_t.
Ananth, install_breakpoint() already has a variable of type addr of type
unsigned long. Why dont you use addr instead of vaddr.
--
Thanks and regards
Srikar
From: Ananth N Mavinakayanahalli <redacted>
On RISC architectures like powerpc, instructions are fixed size.
Instruction analysis on such platforms is just a matter of (insn % 4).
Pass the vaddr at which the uprobe is to be inserted so that
arch_uprobe_analyze_insn() can flag misaligned registration requests.
And the next patch checks "vaddr & 0x03".
But why do you need this new arg? arch_uprobe_analyze_insn() could
check "container_of(auprobe, struct uprobe, arch)->offset & 0x3" with
the same effect, no? vm_start/vm_pgoff are obviously page-aligned.
Oleg.
From: Ananth N Mavinakayanahalli <redacted>
On RISC architectures like powerpc, instructions are fixed size.
Instruction analysis on such platforms is just a matter of (insn % 4).
Pass the vaddr at which the uprobe is to be inserted so that
arch_uprobe_analyze_insn() can flag misaligned registration requests.
And the next patch checks "vaddr & 0x03".
But why do you need this new arg? arch_uprobe_analyze_insn() could
check "container_of(auprobe, struct uprobe, arch)->offset & 0x3" with
the same effect, no? vm_start/vm_pgoff are obviously page-aligned.
We cant use container_of because we moved the definition for struct
uprobe to kernel/events/uprobe.c. This was possible before when struct
uprobe definition was in include/uprobes.h
--
Thanks and Regards
Srikar
From: Jim Keniston <hidden> Date: 2012-06-06 18:08:15
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
quoted
One TODO in this port compared to x86 is the uprobe abort_xol() logic.
x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
if a signal was caused when the uprobed instruction was single-stepped/
emulated, in which case, we reset the instruction pointer to the probed
address and retry the probe again.
Another curious difference is that x86 uses an instruction decoder and
contains massive tables to validate we can probe a particular
instruction.
Part of that difference is because the x86 instruction set is a lot more
complex. Another part is due to the lack, back when the x86 code was
created, of robust handling by uprobes of traps by probed instructions.
So we refused to probe instructions that we knew (or strongly suspected)
would generate traps in user mode -- e.g., privileged instructions,
illegal instructions. A couple of times we had to "legalize"
instructions or prefixes that we didn't originally expect to encounter.
quoted
Can we probe all possible PPC instructions?
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
I am not aware of specific caveats for vector/altivec instructions;
maybe Paul or Ben are more suitable to comment on that.
Ananth
Don't we traditionally use unsigned long to pass vaddrs?
Right. But the vaddr we pass here is vma_info->vaddr which is loff_t.
I guess I should've made that clear in the patch description.
Why not fix struct vma_info's vaddr type?
Calculating and comparing vaddr results either uses variables of type loff_t.
To avoid typecasting and avoid overflow at each of these places, we used
loff_t.
Ananth, install_breakpoint() already has a variable of type addr of type
unsigned long. Why dont you use addr instead of vaddr.
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-08 04:36:32
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
...
quoted
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
Ananth
From: Michael Ellerman <hidden> Date: 2012-06-08 05:52:00
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
quoted
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
...
quoted
quoted
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
But you're not executing the instruction, you're passing it to
emulate_step(). Or am I missing something?
cheers
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-08 06:07:45
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
quoted
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
...
quoted
quoted
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
But you're not executing the instruction, you're passing it to
emulate_step(). Or am I missing something?
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Ananth
From: Michael Ellerman <hidden> Date: 2012-06-08 06:17:47
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
quoted
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
...
quoted
quoted
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
But you're not executing the instruction, you're passing it to
emulate_step(). Or am I missing something?
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
cheers
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-08 06:20:29
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
quoted
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
quoted
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
...
quoted
quoted
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
But you're not executing the instruction, you're passing it to
emulate_step(). Or am I missing something?
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
From: Michael Ellerman <hidden> Date: 2012-06-08 06:38:19
On Fri, 2012-06-08 at 11:49 +0530, Ananth N Mavinakayanahalli wrote:
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
Sure. Will add that check and send v2.
Sorry I didn't mean add a test in the code, I meant construct a test
case to confirm that it works as expected.
cheers
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-08 09:21:13
On Fri, Jun 08, 2012 at 04:38:17PM +1000, Michael Ellerman wrote:
On Fri, 2012-06-08 at 11:49 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
Sure. Will add that check and send v2.
Sorry I didn't mean add a test in the code, I meant construct a test
case to confirm that it works as expected.
Michael,
I just hand-coded the instr to emulate_step() and here are the results:
MSR_PR is set
insn = 7c600124, ret = 0 /* mtmsr */
insn = 7c600164, ret = 0 /* mtmsrd */
insn = 4c000024, ret = -1 /* rfid */
insn = 4c000064, ret = 0 /* rfi */
Also verified that standalone programs with those instructions in inline
asm will die with a SIGILL.
So, for mtmsr, mtmsrd and rfi, we have to single-step them which will
result in a SIGILL in turn.
Ananth
From: Michael Ellerman <hidden> Date: 2012-06-12 04:01:56
On Fri, 2012-06-08 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
On Fri, Jun 08, 2012 at 04:38:17PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:49 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
Sure. Will add that check and send v2.
Sorry I didn't mean add a test in the code, I meant construct a test
case to confirm that it works as expected.
Michael,
I just hand-coded the instr to emulate_step() and here are the results:
MSR_PR is set
insn = 7c600124, ret = 0 /* mtmsr */
insn = 7c600164, ret = 0 /* mtmsrd */
insn = 4c000024, ret = -1 /* rfid */
insn = 4c000064, ret = 0 /* rfi */
Also verified that standalone programs with those instructions in inline
asm will die with a SIGILL.
So, for mtmsr, mtmsrd and rfi, we have to single-step them which will
result in a SIGILL in turn.
What happens in the rfid case? You don't handle -1 from emulate_step()
any differently AFAICS, so don't we try to single step that too?
cheers
From: Ananth N Mavinakayanahalli <hidden> Date: 2012-06-12 04:52:59
On Tue, Jun 12, 2012 at 02:01:46PM +1000, Michael Ellerman wrote:
On Fri, 2012-06-08 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 04:38:17PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:49 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
quoted
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
quoted
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
Sure. Will add that check and send v2.
Sorry I didn't mean add a test in the code, I meant construct a test
case to confirm that it works as expected.
Michael,
I just hand-coded the instr to emulate_step() and here are the results:
MSR_PR is set
insn = 7c600124, ret = 0 /* mtmsr */
insn = 7c600164, ret = 0 /* mtmsrd */
insn = 4c000024, ret = -1 /* rfid */
insn = 4c000064, ret = 0 /* rfi */
Also verified that standalone programs with those instructions in inline
asm will die with a SIGILL.
So, for mtmsr, mtmsrd and rfi, we have to single-step them which will
result in a SIGILL in turn.
What happens in the rfid case? You don't handle -1 from emulate_step()
any differently AFAICS, so don't we try to single step that too?
-1 is just emulate_step() flagging cases where instructions must not be
single-stepped (rfi[d], mtmsr that clears MSR_RI). But as with the other
OEA instructions in user space, we fail with a SIGILL.
As the application is hozed in any case if we encounter an OEA
instruction, I'd think there is no point in handling a -1 from
emulate_step() any differently.
Ananth