Re: [PATCH -next V11 1/3] riscv: stack: Support HAVE_IRQ_EXIT_ON_IRQ_STACK

[Guo Ren]

On Mon, Mar 27, 2023 at 7:30 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> On Fri, Mar 24, 2023 at 03:12:37AM -0400, guoren@xxxxxxxxxx wrote:
> > From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> >
> > Add independent irq stacks for percpu to prevent kernel stack overflows.
> > It is also compatible with VMAP_STACK by implementing
> > arch_alloc_vmap_stack.  Many architectures have supported
> > HAVE_IRQ_EXIT_ON_IRQ_STACK, riscv should follow up.
> >
> > Tested-by: Jisheng Zhang <jszhang@xxxxxxxxxx>
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxx>
>
> > --- a/arch/riscv/kernel/irq.c
> > +++ b/arch/riscv/kernel/irq.c
> > @@ -9,6 +9,37 @@
> >  #include <linux/irqchip.h>
> >  #include <linux/seq_file.h>
> >  #include <asm/smp.h>
> > +#include <asm/vmap_stack.h>
> > +
> > +#ifdef CONFIG_IRQ_STACKS
> > +DEFINE_PER_CPU(ulong *, irq_stack_ptr);
>
> btw, sparse is complaining about this variable:
> ../arch/riscv/kernel/irq.c:15:1: warning: symbol '__pcpu_scope_irq_stack_ptr' was not declared. Should it be static?
I declared it in traps.c, maybe I should put it in the vmap_stack.h.

>
> I'm not immediately sure why that is the case, but should be
> reproducible with gcc-12 allmodconfig.
>
> Thanks,
> Conor.
>
> > +
> > +#ifdef CONFIG_VMAP_STACK
> > +static void init_irq_stacks(void)
> > +{
> > +     int cpu;
> > +     ulong *p;
> > +
> > +     for_each_possible_cpu(cpu) {
> > +             p = arch_alloc_vmap_stack(IRQ_STACK_SIZE, cpu_to_node(cpu));
> > +             per_cpu(irq_stack_ptr, cpu) = p;
> > +     }
> > +}
> > +#else
> > +/* irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned. */
> > +DEFINE_PER_CPU_ALIGNED(ulong [IRQ_STACK_SIZE/sizeof(ulong)], irq_stack);
> > +
> > +static void init_irq_stacks(void)
> > +{
> > +     int cpu;
> > +
> > +     for_each_possible_cpu(cpu)
> > +             per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
> > +}
> > +#endif /* CONFIG_VMAP_STACK */
> > +#else
> > +static void init_irq_stacks(void) {}
> > +#endif /* CONFIG_IRQ_STACKS */
> >
> >  int arch_show_interrupts(struct seq_file *p, int prec)
> >  {
> > @@ -18,6 +49,7 @@ int arch_show_interrupts(struct seq_file *p, int prec)
> >
> >  void __init init_IRQ(void)
> >  {
> > +     init_irq_stacks();
> >       irqchip_init();
> >       if (!handle_arch_irq)
> >               panic("No interrupt controller found.");
> > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> > index 1f4e37be7eb3..b69933ab6bf8 100644
> > --- a/arch/riscv/kernel/traps.c
> > +++ b/arch/riscv/kernel/traps.c
> > @@ -305,16 +305,50 @@ asmlinkage __visible noinstr void do_page_fault(struct pt_regs *regs)
> >  }
> >  #endif
> >
> > -asmlinkage __visible noinstr void do_irq(struct pt_regs *regs)
> > +static void noinstr handle_riscv_irq(struct pt_regs *regs)
> >  {
> >       struct pt_regs *old_regs;
> > -     irqentry_state_t state = irqentry_enter(regs);
> >
> >       irq_enter_rcu();
> >       old_regs = set_irq_regs(regs);
> >       handle_arch_irq(regs);
> >       set_irq_regs(old_regs);
> >       irq_exit_rcu();
> > +}
> > +
> > +#ifdef CONFIG_IRQ_STACKS
> > +DECLARE_PER_CPU(ulong *, irq_stack_ptr);
> > +#endif
I declared it here.

> > +
> > +asmlinkage void noinstr do_irq(struct pt_regs *regs)
> > +{
> > +     irqentry_state_t state = irqentry_enter(regs);
> > +#ifdef CONFIG_IRQ_STACKS
> > +     if (on_thread_stack()) {
> > +             ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
> > +                                     + IRQ_STACK_SIZE/sizeof(ulong);
> > +             __asm__ __volatile(
> > +             "addi   sp, sp, -"RISCV_SZPTR  "\n"
> > +             REG_S"  ra, (sp)                \n"
> > +             "addi   sp, sp, -"RISCV_SZPTR  "\n"
> > +             REG_S"  s0, (sp)                \n"
> > +             "addi   s0, sp, 2*"RISCV_SZPTR "\n"
> > +             "move   sp, %[sp]               \n"
> > +             "move   a0, %[regs]             \n"
> > +             "call   handle_riscv_irq        \n"
> > +             "addi   sp, s0, -2*"RISCV_SZPTR"\n"
> > +             REG_L"  s0, (sp)                \n"
> > +             "addi   sp, sp, "RISCV_SZPTR   "\n"
> > +             REG_L"  ra, (sp)                \n"
> > +             "addi   sp, sp, "RISCV_SZPTR   "\n"
> > +             :
> > +             : [sp] "r" (sp), [regs] "r" (regs)
> > +             : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
> > +               "t0", "t1", "t2", "t3", "t4", "t5", "t6",
> > +               "memory");
> > +     } else
> > +#endif
> > +             handle_riscv_irq(regs);
> >
> >       irqentry_exit(regs, state);
> >  }
> > --
> > 2.36.1
> >
> >



-- 
Best Regards
 Guo Ren