Re: [PATCH v3 2/4] riscv: add support for SBI Supervisor Software Events extension

[Alexandre Ghiti]

Hi Clément,

On 28/01/2025 09:10, Clément Léger wrote:
> On 27/01/2025 09:09, Alexandre Ghiti wrote:
> > > I believe the goal is not the same. Using CONFIG_VMAP_STACK allows the
> > > kernel exception handling to catch any stack overflow when entering the
> > > kernel and thus using vmalloc is required to allocate twice the page
> > > size (overflow is when sp is located in the upper half of the allocated
> > > vmalloc stack. So basically, this is two distinct purposes.
> > >
> > > AFAIU, kvmalloc allows to fallback to vmalloc if kmalloc fails. This is
> > > not what we are looking for here since our allocation size is always
> > > quite small and known (STACK_SIZE basically).
> > >
> > > But I might be missing something.
> >
> > arch_alloc_vmap_stack() only vmalloc the stack and does not implement
> > any stack overflow mechanism, so I'm still unsure we need the define.
> Hi Alex,
>
> So actually, the stack overflow check itself is done in the exception
> entry. It check if the stack pointer did passed in the upper part of the
> vmalloc allocation (see entry.S:122). In this allocation, the stack size
> is actually * 2:
>
> #ifdef CONFIG_VMAP_STACK
> #define THREAD_ALIGN            (2 * THREAD_SIZE)
> #else
> #define THREAD_ALIGN            THREAD_SIZE
> #endif
>
> So even though it does nothing special by itself, it centralize the
> allocation size/method. And size the size is larger, using vamlloc makes
> sense I guess. The same mechanism is used to allocate irq stack as well.


You're right, it makes sense! Nit: we can avoid the ifdef by using 
IS_ENABLED() but do as you prefer.

Thanks for the explanation,

Alex


> Thanks,
>
> Clément
>
> > Thanks,
> >
> > Alex