| Message ID | 20191126052141.28009-16-jniethe5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Initial Prefixed Instruction support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (2ec2260ce7bce5eb6a8ced0bb78d75c1b3eca306) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 48 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Tue, Nov 26, 2019 at 04:21:38PM +1100, Jordan Niethe wrote: > Uprobes can execute instructions out of line. Increase the size of the > buffer used for this so that this works for prefixed instructions. Take > into account the length of prefixed instructions when fixing up the nip. > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > arch/powerpc/include/asm/uprobes.h | 18 ++++++++++++++---- > arch/powerpc/kernel/uprobes.c | 4 ++-- > 2 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h > index 2bbdf27d09b5..5b5e8a3d2f55 100644 > --- a/arch/powerpc/include/asm/uprobes.h > +++ b/arch/powerpc/include/asm/uprobes.h > @@ -14,18 +14,28 @@ > > typedef ppc_opcode_t uprobe_opcode_t; > > +/* > + * We have to ensure we have enought space for prefixed instructions, which minor typo of `enought` and we can have something like below, s/We have to ensure we have enought/Ensure we have enough -- Bala > + * are double the size of a word instruction, i.e. 8 bytes. However, > + * sometimes it is simpler to treat a prefixed instruction like 2 word > + * instructions. > + */ > #define MAX_UINSN_BYTES 4 > -#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES) > +#define UPROBE_XOL_SLOT_BYTES (2 * MAX_UINSN_BYTES) > > /* The following alias is needed for reference from arch-agnostic code */ > #define UPROBE_SWBP_INSN BREAKPOINT_INSTRUCTION > #define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */ > > struct arch_uprobe { > + /* > + * Ensure there is enough space for prefixed instructions. Prefixed > + * instructions must not cross 64-byte boundaries. > + */ > union { > - u32 insn; > - u32 ixol; > - }; > + uprobe_opcode_t insn[2]; > + uprobe_opcode_t ixol[2]; > + } __aligned(64); > }; > > struct arch_uprobe_task { > diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c > index ab1077dc6148..cfcea6946f8b 100644 > --- a/arch/powerpc/kernel/uprobes.c > +++ b/arch/powerpc/kernel/uprobes.c > @@ -111,7 +111,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > * support doesn't exist and have to fix-up the next instruction > * to be executed. > */ > - regs->nip = utask->vaddr + MAX_UINSN_BYTES; > + regs->nip = utask->vaddr + ((IS_PREFIX(auprobe->insn[0])) ? 8 : 4); > > user_disable_single_step(current); > return 0; > @@ -173,7 +173,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) > * emulate_step() returns 1 if the insn was successfully emulated. > * For all other cases, we need to single-step in hardware. > */ > - ret = emulate_step(regs, auprobe->insn, 0); > + ret = emulate_step(regs, auprobe->insn[0], auprobe->insn[1]); > if (ret > 0) > return true; > > -- > 2.20.1 >
On Mon, Jan 13, 2020 at 10:30 PM Balamuruhan S <bala24@linux.ibm.com> wrote: > > On Tue, Nov 26, 2019 at 04:21:38PM +1100, Jordan Niethe wrote: > > Uprobes can execute instructions out of line. Increase the size of the > > buffer used for this so that this works for prefixed instructions. Take > > into account the length of prefixed instructions when fixing up the nip. > > > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > > --- > > arch/powerpc/include/asm/uprobes.h | 18 ++++++++++++++---- > > arch/powerpc/kernel/uprobes.c | 4 ++-- > > 2 files changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h > > index 2bbdf27d09b5..5b5e8a3d2f55 100644 > > --- a/arch/powerpc/include/asm/uprobes.h > > +++ b/arch/powerpc/include/asm/uprobes.h > > @@ -14,18 +14,28 @@ > > > > typedef ppc_opcode_t uprobe_opcode_t; > > > > +/* > > + * We have to ensure we have enought space for prefixed instructions, which > > minor typo of `enought` and we can have something like below, Thanks for catching that. > > s/We have to ensure we have enought/Ensure we have enough Will do. > > -- Bala > > > + * are double the size of a word instruction, i.e. 8 bytes. However, > > + * sometimes it is simpler to treat a prefixed instruction like 2 word > > + * instructions. > > + */ > > #define MAX_UINSN_BYTES 4 > > -#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES) > > +#define UPROBE_XOL_SLOT_BYTES (2 * MAX_UINSN_BYTES) > > > > /* The following alias is needed for reference from arch-agnostic code */ > > #define UPROBE_SWBP_INSN BREAKPOINT_INSTRUCTION > > #define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */ > > > > struct arch_uprobe { > > + /* > > + * Ensure there is enough space for prefixed instructions. Prefixed > > + * instructions must not cross 64-byte boundaries. > > + */ > > union { > > - u32 insn; > > - u32 ixol; > > - }; > > + uprobe_opcode_t insn[2]; > > + uprobe_opcode_t ixol[2]; > > + } __aligned(64); > > }; > > > > struct arch_uprobe_task { > > diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c > > index ab1077dc6148..cfcea6946f8b 100644 > > --- a/arch/powerpc/kernel/uprobes.c > > +++ b/arch/powerpc/kernel/uprobes.c > > @@ -111,7 +111,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > > * support doesn't exist and have to fix-up the next instruction > > * to be executed. > > */ > > - regs->nip = utask->vaddr + MAX_UINSN_BYTES; > > + regs->nip = utask->vaddr + ((IS_PREFIX(auprobe->insn[0])) ? 8 : 4); > > > > user_disable_single_step(current); > > return 0; > > @@ -173,7 +173,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) > > * emulate_step() returns 1 if the insn was successfully emulated. > > * For all other cases, we need to single-step in hardware. > > */ > > - ret = emulate_step(regs, auprobe->insn, 0); > > + ret = emulate_step(regs, auprobe->insn[0], auprobe->insn[1]); > > if (ret > 0) > > return true; > > > > -- > > 2.20.1 > > >
diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h index 2bbdf27d09b5..5b5e8a3d2f55 100644 --- a/arch/powerpc/include/asm/uprobes.h +++ b/arch/powerpc/include/asm/uprobes.h @@ -14,18 +14,28 @@ typedef ppc_opcode_t uprobe_opcode_t; +/* + * We have to ensure we have enought space for prefixed instructions, which + * are double the size of a word instruction, i.e. 8 bytes. However, + * sometimes it is simpler to treat a prefixed instruction like 2 word + * instructions. + */ #define MAX_UINSN_BYTES 4 -#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES) +#define UPROBE_XOL_SLOT_BYTES (2 * MAX_UINSN_BYTES) /* The following alias is needed for reference from arch-agnostic code */ #define UPROBE_SWBP_INSN BREAKPOINT_INSTRUCTION #define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */ struct arch_uprobe { + /* + * Ensure there is enough space for prefixed instructions. Prefixed + * instructions must not cross 64-byte boundaries. + */ union { - u32 insn; - u32 ixol; - }; + uprobe_opcode_t insn[2]; + uprobe_opcode_t ixol[2]; + } __aligned(64); }; struct arch_uprobe_task { diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index ab1077dc6148..cfcea6946f8b 100644 --- a/arch/powerpc/kernel/uprobes.c +++ b/arch/powerpc/kernel/uprobes.c @@ -111,7 +111,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) * support doesn't exist and have to fix-up the next instruction * to be executed. */ - regs->nip = utask->vaddr + MAX_UINSN_BYTES; + regs->nip = utask->vaddr + ((IS_PREFIX(auprobe->insn[0])) ? 8 : 4); user_disable_single_step(current); return 0; @@ -173,7 +173,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) * emulate_step() returns 1 if the insn was successfully emulated. * For all other cases, we need to single-step in hardware. */ - ret = emulate_step(regs, auprobe->insn, 0); + ret = emulate_step(regs, auprobe->insn[0], auprobe->insn[1]); if (ret > 0) return true;
Uprobes can execute instructions out of line. Increase the size of the buffer used for this so that this works for prefixed instructions. Take into account the length of prefixed instructions when fixing up the nip. Signed-off-by: Jordan Niethe <jniethe5@gmail.com> --- arch/powerpc/include/asm/uprobes.h | 18 ++++++++++++++---- arch/powerpc/kernel/uprobes.c | 4 ++-- 2 files changed, 16 insertions(+), 6 deletions(-)