Messages in this thread | | | From | Guo Ren <> | Date | Wed, 11 Jan 2023 21:22:09 +0800 | Subject | Re: [PATCH -next V6 1/7] riscv: ftrace: Fixup panic by disabling preemption |
| |
On Tue, Jan 10, 2023 at 1:20 AM Mark Rutland <mark.rutland@arm.com> wrote: > > On Sat, Jan 07, 2023 at 08:35:43AM -0500, guoren@kernel.org wrote: > > From: Andy Chiu <andy.chiu@sifive.com> > > > > In RISCV, we must use an AUIPC + JALR pair to encode an immediate, > > forming a jump that jumps to an address over 4K. This may cause errors > > if we want to enable kernel preemption and remove dependency from > > patching code with stop_machine(). For example, if a task was switched > > out on auipc. And, if we changed the ftrace function before it was > > switched back, then it would jump to an address that has updated 11:0 > > bits mixing with previous XLEN:12 part. > > > > 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 > > p| REG_L ra, -SZREG(sp) > > func: > > xxx > > ret > > What happens on SMP but not !PREEMPTION; can't a CPU be in the middle of this > while you're patching the sequence? Yes, when PREEMPTION, a timer interrupt between auipc & jalr may cause context_switch. And riscv uses stop_machine for patch_text. Then, we may modify auipc part, but only execute the jalr part when return.
> > Do you have any guarantee as to the atomicity and ordering of instruction > fetches? Not yet. If the region is short, we could use nop + jalr pair instead. Only one jalr instruction makes the entry atomicity.
There are already several proposed solutions: 1. Make stop_machine guarantee all CPU out of preemption point. 2. Expand -fpatchable-function-entry from 4 to 24, and make detour codes atomicity. 3. We want to propose a solution to make auipc by hardware mask_irq. For more details, see: https://www.youtube.com/watch?v=4JkkkXuEvCw
> > Thanks, > Mark. > > > > > Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > --- > > arch/riscv/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index e2b656043abf..ee0d39b26794 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -138,7 +138,7 @@ config RISCV > > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > select HAVE_FUNCTION_GRAPH_TRACER > > - select HAVE_FUNCTION_TRACER if !XIP_KERNEL > > + select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > config ARCH_MMAP_RND_BITS_MIN > > default 18 if 64BIT > > -- > > 2.36.1 > >
-- Best Regards Guo Ren
| |