This can be applied on top of Petr Mladek's v4 rework of the ppc64le
live patching. Inspired by Balbir Singh's v5, information about the
callee's r2 is stored in a "reserved" 32 bit location in the caller's
stack frame, instead of 64 bits in the newly created mini frame 24(r1).
It only needs to work for a local call, when caller's TOC == callee's
TOC, and along with the return address (LR) it's all within a 4GiB
range (+-31 bits). If the original call already was global, we are
allowed to restore any nonsense into r2, because the global caller
will restore its TOC anyway from the ABI compliant location 24(r1)
right after return.
Signed-off-by: Torsten Duwe <[email protected]>
---
This is only the preparation for dumping the mini stack frame.
It shouldn't break anything, bisecting-wise.
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1284,8 +1284,9 @@ ftrace_call:
* lr and LRSAVE(r1) contain the address of klp_return_helper.
* We loaded ctr with the address of the patched function earlier
*/
+ subf r0, r0, r2 /* Calculate offset from current TOC to LR */
+ stw r0, 12(r1) /* and save it in CR+4 */
stdu r1, -32(r1) /* open new mini stack frame */
- std r2, 24(r1) /* save TOC now, unconditionally. */
bl 5f
5: mflr r12
addi r12, r12, (klp_return_helper + 4 - .)@l
@@ -1323,9 +1324,10 @@ _GLOBAL(ftrace_stub)
* maybe inserting a klp_return_helper frame or not.
*/
klp_return_helper:
- ld r2, 24(r1) /* restore TOC (saved by ftrace_caller) */
addi r1, r1, 32 /* destroy mini stack frame */
+ lwa r2, 12(r1) /* Load from CR+4, offset of TOC w.r.t LR */
ld r0, LRSAVE(r1) /* get the real return address */
+ add r2, r2, r0 /* Add the current LR to offset */
mtlr r0
blr
#endif
After the mini stack frame is no longer required for TOC storage, it can
be eliminated iff the functionality of klp_return_helper, which required
a stack frame for the extra return address previously, is carried out
by the replacement function now. This requires _every_ live patch replacement
function to execute the following (or similar) sequence of machine instructions
just before every return to the original caller:
ld r0, 0(r1) /* use back link to find caller's frame */
lwa r2, 12(r0) /* Load from CR+4, offset of TOC w.r.t LR */
ld r0, LRSAVE(r0) /* get the real return address */
add r2, r2, r0 /* Add the current LR to offset */
Signed-off-by: Torsten Duwe <[email protected]>
---
This is solution 1 now.
Do we really want that? I don't think so; this is merely to illustrate
what the alternative to klp_return_helper and its extra stack frame would
look like. Hence, I didn't test yet whether all the details are correct.
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1277,21 +1277,11 @@ ftrace_call:
* For a local call, restore this TOC after calling the patch function.
* For a global call, it does not matter what we restore here,
* since the global caller does its own restore right afterwards,
- * anyway. Just insert a klp_return_helper frame in any case,
- * so a patch function can always count on the changed stack offsets.
- * The patch introduces a frame such that from the patched function
- * we return back to klp_return helper. For ABI compliance r12,
- * lr and LRSAVE(r1) contain the address of klp_return_helper.
- * We loaded ctr with the address of the patched function earlier
+ * anyway. Just prepare here the TOC restore in patch functions.
+ * We loaded ctr with the address of the patched function earlier.
*/
subf r0, r0, r2 /* Calculate offset from current TOC to LR */
stw r0, 12(r1) /* and save it in CR+4 */
- stdu r1, -32(r1) /* open new mini stack frame */
- bl 5f
-5: mflr r12
- addi r12, r12, (klp_return_helper + 4 - .)@l
- std r12, LRSAVE(r1)
- mtlr r12
mfctr r12 /* allow for TOC calculation in newfunc */
bctr
4:
@@ -1313,25 +1303,6 @@ _GLOBAL(ftrace_graph_stub)
_GLOBAL(ftrace_stub)
blr
-#ifdef CONFIG_LIVEPATCH
-/* Helper function for local calls that are becoming global
- * due to live patching.
- * We can't simply patch the NOP after the original call,
- * because, depending on the consistency model, some kernel
- * threads may still have called the original, local function
- * *without* saving their TOC in the respective stack frame slot,
- * so the decision is made per-thread during function return by
- * maybe inserting a klp_return_helper frame or not.
-*/
-klp_return_helper:
- addi r1, r1, 32 /* destroy mini stack frame */
- lwa r2, 12(r1) /* Load from CR+4, offset of TOC w.r.t LR */
- ld r0, LRSAVE(r1) /* get the real return address */
- add r2, r2, r0 /* Add the current LR to offset */
- mtlr r0
- blr
-#endif
-
#else
_GLOBAL_TOC(_mcount)
On Wed 2016-03-09 18:30:17, Torsten Duwe wrote:
> After the mini stack frame is no longer required for TOC storage, it can
> be eliminated iff the functionality of klp_return_helper, which required
> a stack frame for the extra return address previously, is carried out
> by the replacement function now. This requires _every_ live patch replacement
> function to execute the following (or similar) sequence of machine instructions
> just before every return to the original caller:
I have thought about it and it is a nono from my point of view.
It is too error prone, especially that there are functions that
call return on several locations.
Best Regards,
Petr
On Thu 2016-03-10 13:25:08, Petr Mladek wrote:
> On Wed 2016-03-09 18:30:17, Torsten Duwe wrote:
> > After the mini stack frame is no longer required for TOC storage, it can
> > be eliminated iff the functionality of klp_return_helper, which required
> > a stack frame for the extra return address previously, is carried out
> > by the replacement function now. This requires _every_ live patch replacement
> > function to execute the following (or similar) sequence of machine instructions
> > just before every return to the original caller:
>
> I have thought about it and it is a nono from my point of view.
> It is too error prone, especially that there are functions that
> call return on several locations.
BTW: How is this solved in kretprobes? Or is it easier there?
Best Regards,
Petr
On Thu, 10 Mar 2016, Petr Mladek wrote:
> BTW: How is this solved in kretprobes? Or is it easier there?
Is that really a problem? With kretprobes you're never performing the
module->core->module transition in "one" redirection, right?
The 'kretprobe_trampoline' is a global symbol, and hence everybody is
generating 'global call' for it, and that's pretty much it.
--
Jiri Kosina
SUSE Labs
On Thu, Mar 10, 2016 at 01:51:16PM +0100, Petr Mladek wrote:
> On Thu 2016-03-10 13:25:08, Petr Mladek wrote:
> > On Wed 2016-03-09 18:30:17, Torsten Duwe wrote:
> > > After the mini stack frame is no longer required for TOC storage, it can
> > > be eliminated iff the functionality of klp_return_helper, which required
> > > a stack frame for the extra return address previously, is carried out
> > > by the replacement function now. This requires _every_ live patch replacement
> > > function to execute the following (or similar) sequence of machine instructions
> > > just before every return to the original caller:
> >
> > I have thought about it and it is a nono from my point of view.
> > It is too error prone, especially that there are functions that
> > call return on several locations.
Yes, that's what I think as well when I look at it.
> BTW: How is this solved in kretprobes? Or is it easier there?
Without any look at the code I assume it uses solution 3. Once
you have a probing framework in place, you can remember the real
return addresses in a data structure. As I wrote, the function
graph tracer does it this way so it would be abvious.
Torsten
On Thu, 2016-03-10 at 14:04 +0100, Torsten Duwe wrote:
> On Thu, Mar 10, 2016 at 01:51:16PM +0100, Petr Mladek wrote:
> > On Thu 2016-03-10 13:25:08, Petr Mladek wrote:
> > > On Wed 2016-03-09 18:30:17, Torsten Duwe wrote:
> > > > After the mini stack frame is no longer required for TOC storage, it can
> > > > be eliminated iff the functionality of klp_return_helper, which required
> > > > a stack frame for the extra return address previously, is carried out
> > > > by the replacement function now. This requires _every_ live patch replacement
> > > > function to execute the following (or similar) sequence of machine instructions
> > > > just before every return to the original caller:
> > >
> > > I have thought about it and it is a nono from my point of view.
> > > It is too error prone, especially that there are functions that
> > > call return on several locations.
>
> Yes, that's what I think as well when I look at it.
> > BTW: How is this solved in kretprobes? Or is it easier there?
>
> Without any look at the code I assume it uses solution 3. Once
> you have a probing framework in place, you can remember the real
> return addresses in a data structure. As I wrote, the function
> graph tracer does it this way so it would be abvious.
Yeah it has a linked list of struct kretprobe_instance's, each of which stores
the real return address.
I have some ideas for how to fix livepatch, but this week is a bit busy with
merge window prep.
cheers
On 10/03/16 04:28, Torsten Duwe wrote:
> This can be applied on top of Petr Mladek's v4 rework of the ppc64le
> live patching. Inspired by Balbir Singh's v5, information about the
> callee's r2 is stored in a "reserved" 32 bit location in the caller's
> stack frame, instead of 64 bits in the newly created mini frame 24(r1).
>
> It only needs to work for a local call, when caller's TOC == callee's
> TOC, and along with the return address (LR) it's all within a 4GiB
> range (+-31 bits). If the original call already was global, we are
> allowed to restore any nonsense into r2, because the global caller
> will restore its TOC anyway from the ABI compliant location 24(r1)
> right after return.
>
>
Hi, Torsten
Sorry, I've had no time to test this. Caught up with something else for the moment.
Hopefully I'll get a chance over the weekend.
Have you tested this against Petr's sample changes to patch printk?
Balbir Singh.