lkml.org 
[lkml]   [2020]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks
On Mon, Nov 30, 2020 at 01:13:07PM -0800, Sami Tolvanen wrote:
> On Mon, Nov 30, 2020 at 3:49 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote:
> > > Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of
> > > using statically allocated stacks.
> > >
> > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> > > ---
> > > arch/arm64/kernel/Makefile | 1 -
> > > arch/arm64/kernel/entry.S | 6 ++--
> > > arch/arm64/kernel/irq.c | 19 ++++++++++
> > > arch/arm64/kernel/scs.c | 16 ---------
> > > arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++-------
> > > include/linux/scs.h | 4 ---
> > > 6 files changed, 81 insertions(+), 36 deletions(-)
> > > delete mode 100644 arch/arm64/kernel/scs.c
> >
> > [...]
> >
> > > @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu)
> > > return 0;
> > > }
> > >
> > > +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu)
> > > +{
> > > + void *s;
> > > +
> > > + s = scs_alloc(cpu_to_node(cpu));
> > > + if (!s)
> > > + return -ENOMEM;
> > > + per_cpu(*ptr, cpu) = s;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int init_sdei_stacks(void)
> > > {
> > > int cpu;
> > > int err = 0;
> > >
> > > for_each_possible_cpu(cpu) {
> > > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu);
> > > - if (err)
> > > - break;
> > > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu);
> > > - if (err)
> > > - break;
> > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) {
> > > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu);
> > > + if (err)
> > > + break;
> > > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu);
> > > + if (err)
> > > + break;
> > > + }
> > > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) {
> > > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu);
> > > + if (err)
> > > + break;
> > > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu);
> > > + if (err)
> > > + break;
> >
> > This looks ok to me, but I think it would be better to follow the same
> > approach as you have for the IRQ stacks and instead have a separate
> > init_sdei_scs() function (similarly for the free() path), which means
> > you can simply the IS_ENABLED() checks too.
>
> OK, I can change this in v3. It makes error handling in
> sdei_arch_get_entry_point() a bit more awkward though. We'll need
> something like this:
>
> if (IS_ENABLED(CONFIG_VMAP_STACK)) {
> if (init_sdei_stacks())
> return 0;
> }
>
> if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) {
> if (init_sdei_scs()) {
> if (IS_ENABLED(CONFIG_VMAP_STACK))
> free_sdei_stacks();
> return 0;
> }

Can you push the IS_ENABLED() checks into their respective functions?
Then you can do something like:

if (init_sdei_stacks())
return 0;

if (init_sdei_scs())
goto out_free_stacks;

...

out_free_stacks:
free_sdei_stacks();
return 0;


Will

\
 
 \ /
  Last update: 2020-12-01 11:22    [W:0.074 / U:2.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site