Re: [PATCH v15 08/10] arm64: Add trampoline code for kretprobes

[David Long]

On 07/19/2016 09:46 AM, Catalin Marinas wrote:
On Fri, Jul 08, 2016 at 12:35:52PM -0400, David Long wrote:
--- /dev/null
+++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
@@ -0,0 +1,85 @@
+/*
+ * trampoline entry and return code for kretprobes.
+ */
+
+#include <linux/linkage.h>
+#include <asm/asm-offsets.h>
+#include <asm/assembler.h>
+
+       .text
+
+.macro save_all_base_regs
+       stp x0, x1, [sp, #S_X0]
+       stp x2, x3, [sp, #S_X2]
+       stp x4, x5, [sp, #S_X4]
+       stp x6, x7, [sp, #S_X6]
+       stp x8, x9, [sp, #S_X8]
+       stp x10, x11, [sp, #S_X10]
+       stp x12, x13, [sp, #S_X12]
+       stp x14, x15, [sp, #S_X14]
+       stp x16, x17, [sp, #S_X16]
+       stp x18, x19, [sp, #S_X18]
+       stp x20, x21, [sp, #S_X20]
+       stp x22, x23, [sp, #S_X22]
+       stp x24, x25, [sp, #S_X24]
+       stp x26, x27, [sp, #S_X26]
+       stp x28, x29, [sp, #S_X28]
+       add x0, sp, #S_FRAME_SIZE
+       stp lr, x0, [sp, #S_LR]
+/*
+ * Construct a useful saved PSTATE
+ */
+       mrs x0, nzcv
+       and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
+       mrs x1, daif
+       and x1, x1, #(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)

I don't think you need the masking here, the mrs should return the
corresponding 4 bits.


OK. I see you've done that.

+       orr x0, x0, x1
+       mrs x1, CurrentEL
+       and x1, x1, #(3 << 2)
+       orr x0, x1, x0
+       mrs x1, SPSel
+       and x1, x1, #1

Same here.

OK. ^


+       orr x0, x1, x0
+       str x0, [sp, #S_PSTATE]
+.endm

How is this pstate used, other than the restoring of the condition flag
in the restore_all_base_regs macro? Does a kretprobes handler need
access to them?


A kretprobes handler should probably be able to examine a reasonable 
pstate value, particularly in terms of DAIF. As I recall not having a 
valid DAIF was an issue at one time.

Anyway, it's worth doing an stp xzr, x0, [sp, S_PC] so that we
initialise the pc in pt_regs.


OK.  Looks like you've done this.

+
+.macro restore_all_base_regs
+       ldr x0, [sp, #S_PSTATE]
+       and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
+       msr nzcv, x0
+       ldp x0, x1, [sp, #S_X0]
+       ldp x2, x3, [sp, #S_X2]
+       ldp x4, x5, [sp, #S_X4]
+       ldp x6, x7, [sp, #S_X6]
+       ldp x8, x9, [sp, #S_X8]
+       ldp x10, x11, [sp, #S_X10]
+       ldp x12, x13, [sp, #S_X12]
+       ldp x14, x15, [sp, #S_X14]
+       ldp x16, x17, [sp, #S_X16]
+       ldp x18, x19, [sp, #S_X18]
+       ldp x20, x21, [sp, #S_X20]
+       ldp x22, x23, [sp, #S_X22]
+       ldp x24, x25, [sp, #S_X24]
+       ldp x26, x27, [sp, #S_X26]
+       ldp x28, x29, [sp, #S_X28]
+.endm
+
+ENTRY(kretprobe_trampoline)
+
+       sub sp, sp, #S_FRAME_SIZE
+
+       save_all_base_regs
+
+       mov x0, sp
+       bl trampoline_probe_handler
+       /* Replace trampoline address in lr with actual
+          orig_ret_addr return address. */
+       mov lr, x0
+
+       restore_all_base_regs
+
+       add sp, sp, #S_FRAME_SIZE
+
+       ret
+
+ENDPROC(kretprobe_trampoline)


Thanks,
-dl