[PATCH RFC v2 riscv/for-next 3/5] riscv: ftrace: use indirect jump to work with kernel preemption

[Andy Chiu]

Hi Guo,

On Wed, Sep 14, 2022 at 2:45 PM Guo Ren <guoren at kernel.org> wrote:
>
> I really appreciate you finding the bug, great job.

Thanks, :)

>
> On Tue, Sep 13, 2022 at 5:44 PM Andy Chiu <andy.chiu at sifive.com> wrote:

Consider this case happens on a preemptive kernel, with stop_machine.
And all of stop_machine's sub-functions were marked as no trace.

> > p: patched area performed by dynamic ftrace
> > ftrace_prologue:
> > p|      REG_S   ra, -SZREG(sp)
> > p|      auipc   ra, 0x? ------------> preempted
> >                                         ...
> >                                 change ftrace function
> >                                         ...
> > p|      jalr    -?(ra) <------------- switched back
>
> When auipc + jalr -> nop, is safe, right? Because when switched back,
> jalr -> nop.
> When nop -> auipc + jalr, is buggy, right? Because when switched back,
> nop -> jalr, the ra's value is not expected.
>
> Some machines with instruction fusion won't be affected, because they
> would merge auipc + jalr into one macro-op.

This might not be safe as well, if auipc and jalr happened to sit on a
different cache line. And if there were a cache hit for the line
having the auipc and miss for the jalr after switching back. I do not
really sure if this is possible in practice.

> Qemu shouldn't be broken, because auipc + jalr is always in the same
> tcg block, so no chance for interruption between them.

In fact, qemu is broken. I had not thought of that before I got your
reply. But I believe that there is a size limit for each tcg block,
and the auipc and jalr just locate in a separate tcg block.

Regards,
Andy