Re: [PATCH v1 0/5] Implement livepatch on PPC32

[Steven Rostedt]

On Mon, 13 Dec 2021 17:50:52 +0000
Christophe Leroy <christophe.le...@csgroup.eu> wrote:

> @@ -958,6 +942,12 @@ unsigned long prepare_ftrace_return(unsigned long 
> parent, unsigned long ip,
>   out:
>       return parent;
>   }
> +
> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> +                    struct ftrace_ops *op, struct ftrace_regs *fregs)
> +{
> +     prepare_ftrace_return(ip, kernel_stack_pointer(&fregs->regs), 0);
> +}

I have for powerpc prepare_ftrace_return as:


unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
                                                unsigned long sp)
{
        unsigned long return_hooker;

        if (unlikely(ftrace_graph_is_dead()))
                goto out;

        if (unlikely(atomic_read(&current->tracing_graph_pause)))
                goto out;

        return_hooker = ppc_function_entry(return_to_handler);

        if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp))
                parent = return_hooker;
out:
        return parent;
}

Which means you'll need different parameters to it than what x86 has, which
has the prototype of:

void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
                           unsigned long frame_pointer)

and it does not use the frame_pointer for this case, which is why it is
zero.

For powerpc though, it uses the stack pointer, so you parameters are
incorrect. Looks like it should be:

        prepare_ftrace_return(parent_ip, ip, 
kernel_stack_pointer(&fregs->regs));

And that will likely not be enough. I'll need to update the ctr register,
as that is where the return address is saved. So you'll probably need it to be:

void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
                       struct ftrace_ops *op, struct ftrace_regs *fregs)
{
        unsigned long parent;

        parent = prepare_ftrace_return(parent_ip, ip, 
kernel_stack_pointer(&fregs->regs));
        fregs->regs.ctr = parent;
}



-- Steve